Skip to content

Commit 30b5095

Browse files
dovgopolydovgopoly
authored
feature/group-rbac (#31)
* init RBACGroupable * memory/calldata usage * made hasPermission non-virtual * added names to return params * renamed enum constants & moved enum to contract * reordered inheritance * renamed method * inline opt * optimized storage mappings * optimized * added natspec & typo * typo * renamed init method & added mock * fix * fix & init tests * got rid of enum * typo * fixed * renamed vars * typos * fix mock & coverage 100% * typos * rm import & added dev note
1 parent 7149597 commit 30b5095

File tree

6 files changed

+525
-36
lines changed

6 files changed

+525
-36
lines changed

‎contracts/access-control/RBAC.sol‎

Lines changed: 57 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ abstract contract RBAC is IRBAC, Initializable {
133133
/**
134134
* @notice The function to get the list of user roles
135135
* @param who_ the user
136-
* @return roles_ the roes of the user
136+
* @return roles_ the roles of the user
137137
*/
138138
function getUserRoles(address who_) public view override returns (string[] memory roles_) {
139139
return _userRoles[who_].values();
@@ -181,51 +181,30 @@ abstract contract RBAC is IRBAC, Initializable {
181181
}
182182

183183
/**
184-
* @notice The function to check the user's possesion of the role
184+
* @dev DO NOT call `super.hasPermission(...)` in derived contracts, because this method
185+
* handles not 2 but 3 states: NO PERMISSION, ALLOWED, DISALLOWED
186+
* @notice The function to check the user's possession of the role
185187
* @param who_ the user
186188
* @param resource_ the resource the user has to have the permission of
187189
* @param permission_ the permission the user has to have
188-
* @return true_ if user has the permission, false otherwise
190+
* @return isAllowed_ true if the user has the permission, false otherwise
189191
*/
190192
function hasPermission(
191193
address who_,
192194
string memory resource_,
193195
string memory permission_
194-
) public view override returns (bool) {
195-
StringSet.Set storage _roles = _userRoles[who_];
196+
) public view virtual override returns (bool isAllowed_) {
197+
string[] memory roles_ = getUserRoles(who_);
196198

197-
uint256 length_ = _roles.length();
198-
bool isAllowed_;
199+
for (uint256 i = 0; i < roles_.length; i++) {
200+
string memory role_ = roles_[i];
199201

200-
for (uint256 i = 0; i < length_; i++) {
201-
string memory role_ = _roles.at(i);
202-
203-
StringSet.Set storage _allDisallowed = _rolePermissions[role_][false][ALL_RESOURCE];
204-
StringSet.Set storage _allAllowed = _rolePermissions[role_][true][ALL_RESOURCE];
205-
206-
StringSet.Set storage _disallowed = _rolePermissions[role_][false][resource_];
207-
StringSet.Set storage _allowed = _rolePermissions[role_][true][resource_];
208-
209-
if (
210-
_allDisallowed.contains(ALL_PERMISSION) ||
211-
_allDisallowed.contains(permission_) ||
212-
_disallowed.contains(ALL_PERMISSION) ||
213-
_disallowed.contains(permission_)
214-
) {
202+
if (_isDisallowed(role_, resource_, permission_)) {
215203
return false;
216204
}
217205

218-
if (
219-
_allAllowed.contains(ALL_PERMISSION) ||
220-
_allAllowed.contains(permission_) ||
221-
_allowed.contains(ALL_PERMISSION) ||
222-
_allowed.contains(permission_)
223-
) {
224-
isAllowed_ = true;
225-
}
206+
isAllowed_ = isAllowed_ || _isAllowed(role_, resource_, permission_);
226207
}
227-
228-
return isAllowed_;
229208
}
230209

231210
/**
@@ -296,4 +275,50 @@ abstract contract RBAC is IRBAC, Initializable {
296275

297276
emit RemovedPermissions(role_, resourceToRemove_, permissionsToRemove_, allowed_);
298277
}
278+
279+
/**
280+
* @notice The function to check if the role has the permission
281+
* @param role_ the role to search the permission in
282+
* @param resource_ the role resource to search the permission in
283+
* @param permission_ the permission to search
284+
* @return true_ if the role has the permission, false otherwise
285+
*/
286+
function _isAllowed(
287+
string memory role_,
288+
string memory resource_,
289+
string memory permission_
290+
) internal view returns (bool) {
291+
mapping(string => StringSet.Set) storage _resources = _rolePermissions[role_][true];
292+
293+
StringSet.Set storage _allAllowed = _resources[ALL_RESOURCE];
294+
StringSet.Set storage _allowed = _resources[resource_];
295+
296+
return (_allAllowed.contains(ALL_PERMISSION) ||
297+
_allAllowed.contains(permission_) ||
298+
_allowed.contains(ALL_PERMISSION) ||
299+
_allowed.contains(permission_));
300+
}
301+
302+
/**
303+
* @notice The function to check if the role has the antipermission
304+
* @param role_ the role to search the antipermission in
305+
* @param resource_ the role resource to search the antipermission in
306+
* @param permission_ the antipermission to search
307+
* @return true_ if the role has the antipermission, false otherwise
308+
*/
309+
function _isDisallowed(
310+
string memory role_,
311+
string memory resource_,
312+
string memory permission_
313+
) internal view returns (bool) {
314+
mapping(string => StringSet.Set) storage _resources = _rolePermissions[role_][false];
315+
316+
StringSet.Set storage _allDisallowed = _resources[ALL_RESOURCE];
317+
StringSet.Set storage _disallowed = _resources[resource_];
318+
319+
return (_allDisallowed.contains(ALL_PERMISSION) ||
320+
_allDisallowed.contains(permission_) ||
321+
_disallowed.contains(ALL_PERMISSION) ||
322+
_disallowed.contains(permission_));
323+
}
299324
}

‎contracts/access-control/extensions/RBACGroupable.sol‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity ^0.8.4;
3+
4+
import "../../interfaces/access-control/extensions/IRBACGroupable.sol";
5+
6+
import "../RBAC.sol";
7+
8+
/**
9+
* @notice The Role Based Access Control (RBAC) module
10+
*
11+
* This contract is an extension for the RBAC contract to provide the ability to organize roles
12+
* into groups and assign users to them.
13+
*/
14+
abstract contract RBACGroupable is IRBACGroupable, RBAC {
15+
using StringSet for StringSet.Set;
16+
using SetHelper for StringSet.Set;
17+
18+
mapping(address => StringSet.Set) private _userGroups;
19+
mapping(string => StringSet.Set) private _groupRoles;
20+
21+
/**
22+
* @notice The init function
23+
*/
24+
function __RBACGroupable_init() internal onlyInitializing {
25+
__RBAC_init();
26+
}
27+
28+
/**
29+
* @notice The function to assign the user to groups
30+
* @param who_ the user to be assigned
31+
* @param groupsToAddTo_ the list of groups to assign the user to
32+
*/
33+
function addUserToGroups(
34+
address who_,
35+
string[] memory groupsToAddTo_
36+
) public virtual override onlyPermission(RBAC_RESOURCE, CREATE_PERMISSION) {
37+
require(groupsToAddTo_.length > 0, "RBACGroupable: empty groups");
38+
39+
_addUserToGroups(who_, groupsToAddTo_);
40+
}
41+
42+
/**
43+
* @notice The function to remove the user from groups
44+
* @param who_ the user to be removed from groups
45+
* @param groupsToRemoveFrom_ the list of groups to remove the user from
46+
*/
47+
function removeUserFromGroups(
48+
address who_,
49+
string[] memory groupsToRemoveFrom_
50+
) public virtual override onlyPermission(RBAC_RESOURCE, DELETE_PERMISSION) {
51+
require(groupsToRemoveFrom_.length > 0, "RBACGroupable: empty groups");
52+
53+
_removeUserFromGroups(who_, groupsToRemoveFrom_);
54+
}
55+
56+
/**
57+
* @notice The function to grant roles to the group
58+
* @param groupTo_ the group to grant roles to
59+
* @param rolesToGrant_ the list of roles to grant
60+
*/
61+
function grantGroupRoles(
62+
string memory groupTo_,
63+
string[] memory rolesToGrant_
64+
) public virtual override onlyPermission(RBAC_RESOURCE, CREATE_PERMISSION) {
65+
require(rolesToGrant_.length > 0, "RBACGroupable: empty roles");
66+
67+
_grantGroupRoles(groupTo_, rolesToGrant_);
68+
}
69+
70+
/**
71+
* @notice The function to revoke roles from the group
72+
* @param groupFrom_ the group to revoke roles from
73+
* @param rolesToRevoke_ the list of roles to revoke
74+
*/
75+
function revokeGroupRoles(
76+
string memory groupFrom_,
77+
string[] memory rolesToRevoke_
78+
) public virtual override onlyPermission(RBAC_RESOURCE, DELETE_PERMISSION) {
79+
require(rolesToRevoke_.length > 0, "RBACGroupable: empty roles");
80+
81+
_revokeGroupRoles(groupFrom_, rolesToRevoke_);
82+
}
83+
84+
/**
85+
* @notice The function to get the list of user groups
86+
* @param who_ the user
87+
* @return groups_ the list of user groups
88+
*/
89+
function getUserGroups(address who_) public view override returns (string[] memory groups_) {
90+
return _userGroups[who_].values();
91+
}
92+
93+
/**
94+
* @notice The function to get the list of groups roles
95+
* @param group_ the group
96+
* @return roles_ the list of group roles
97+
*/
98+
function getGroupRoles(
99+
string memory group_
100+
) public view override returns (string[] memory roles_) {
101+
return _groupRoles[group_].values();
102+
}
103+
104+
/**
105+
* @dev DO NOT call `super.hasPermission(...)` in derived contracts, because this method
106+
* handles not 2 but 3 states: NO PERMISSION, ALLOWED, DISALLOWED
107+
* @notice The function to check the user's possession of the role. Unlike the base method,
108+
* this method also looks up the required permission in the user's groups
109+
* @param who_ the user
110+
* @param resource_ the resource the user has to have the permission of
111+
* @param permission_ the permission the user has to have
112+
* @return isAllowed_ true if the user has the permission, false otherwise
113+
*/
114+
function hasPermission(
115+
address who_,
116+
string memory resource_,
117+
string memory permission_
118+
) public view virtual override returns (bool isAllowed_) {
119+
string[] memory roles_ = getUserRoles(who_);
120+
121+
for (uint256 i = 0; i < roles_.length; i++) {
122+
string memory role_ = roles_[i];
123+
124+
if (_isDisallowed(role_, resource_, permission_)) {
125+
return false;
126+
}
127+
128+
isAllowed_ = isAllowed_ || _isAllowed(role_, resource_, permission_);
129+
}
130+
131+
string[] memory groups_ = getUserGroups(who_);
132+
133+
for (uint256 i = 0; i < groups_.length; i++) {
134+
roles_ = getGroupRoles(groups_[i]);
135+
136+
for (uint256 j = 0; j < roles_.length; j++) {
137+
string memory role_ = roles_[j];
138+
139+
if (_isDisallowed(role_, resource_, permission_)) {
140+
return false;
141+
}
142+
143+
isAllowed_ = isAllowed_ || _isAllowed(role_, resource_, permission_);
144+
}
145+
}
146+
}
147+
148+
/**
149+
* @notice The internal function to assign groups to the user
150+
* @param who_ the user to assign groups to
151+
* @param groupsToAddTo_ the list of groups to be assigned
152+
*/
153+
function _addUserToGroups(address who_, string[] memory groupsToAddTo_) internal {
154+
_userGroups[who_].add(groupsToAddTo_);
155+
156+
emit AddedToGroups(who_, groupsToAddTo_);
157+
}
158+
159+
/**
160+
* @notice The internal function to remove the user from groups
161+
* @param who_ the user to be removed from groups
162+
* @param groupsToRemoveFrom_ the list of groups to remove the user from
163+
*/
164+
function _removeUserFromGroups(address who_, string[] memory groupsToRemoveFrom_) internal {
165+
_userGroups[who_].remove(groupsToRemoveFrom_);
166+
167+
emit RemovedFromGroups(who_, groupsToRemoveFrom_);
168+
}
169+
170+
/**
171+
* @notice The internal function to grant roles to the group
172+
* @param groupTo_ the group to grant roles to
173+
* @param rolesToGrant_ the list of roles to grant
174+
*/
175+
function _grantGroupRoles(string memory groupTo_, string[] memory rolesToGrant_) internal {
176+
_groupRoles[groupTo_].add(rolesToGrant_);
177+
178+
emit GrantedGroupRoles(groupTo_, rolesToGrant_);
179+
}
180+
181+
/**
182+
* @notice The internal function to revoke roles from the group
183+
* @param groupFrom_ the group to revoke roles from
184+
* @param rolesToRevoke_ the list of roles to revoke
185+
*/
186+
function _revokeGroupRoles(string memory groupFrom_, string[] memory rolesToRevoke_) internal {
187+
_groupRoles[groupFrom_].remove(rolesToRevoke_);
188+
189+
emit RevokedGroupRoles(groupFrom_, rolesToRevoke_);
190+
}
191+
}

‎contracts/interfaces/access-control/IRBAC.sol‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ interface IRBAC {
2323
bool allowed
2424
);
2525

26-
function grantRoles(address to_, string[] memory rolesToGrant_) external;
26+
function grantRoles(address to_, string[] calldata rolesToGrant_) external;
2727

28-
function revokeRoles(address from_, string[] memory rolesToRevoke_) external;
28+
function revokeRoles(address from_, string[] calldata rolesToRevoke_) external;
2929

3030
function addPermissionsToRole(
3131
string calldata role_,
@@ -39,7 +39,7 @@ interface IRBAC {
3939
bool allowed_
4040
) external;
4141

42-
function getUserRoles(address who_) external view returns (string[] memory roles_);
42+
function getUserRoles(address who_) external view returns (string[] calldata roles_);
4343

4444
function getRolePermissions(
4545
string calldata role_
@@ -55,5 +55,5 @@ interface IRBAC {
5555
address who_,
5656
string calldata resource_,
5757
string calldata permission_
58-
) external view returns (bool);
58+
) external view returns (bool isAllowed_);
5959
}

‎contracts/interfaces/access-control/extensions/IRBACGroupable.sol‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity ^0.8.4;
3+
4+
/**
5+
* @notice The RBAC module
6+
*/
7+
interface IRBACGroupable {
8+
event AddedToGroups(address who, string[] groupsToAddTo);
9+
event RemovedFromGroups(address who, string[] groupsToRemoveFrom);
10+
11+
event GrantedGroupRoles(string groupTo, string[] rolesToGrant);
12+
event RevokedGroupRoles(string groupFrom, string[] rolesToRevoke);
13+
14+
function addUserToGroups(address who_, string[] calldata groupsToAddTo_) external;
15+
16+
function removeUserFromGroups(address who_, string[] calldata groupsToRemoveFrom_) external;
17+
18+
function grantGroupRoles(string calldata groupTo_, string[] calldata rolesToGrant_) external;
19+
20+
function revokeGroupRoles(
21+
string calldata groupFrom_,
22+
string[] calldata rolesToRevoke_
23+
) external;
24+
25+
function getUserGroups(address who_) external view returns (string[] calldata groups_);
26+
27+
function getGroupRoles(
28+
string calldata group_
29+
) external view returns (string[] calldata roles_);
30+
}

‎contracts/mock/access-control/extensions/RBACGroupableMock.sol‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// SPDX-License-Identifier: MIT
2+
pragma solidity ^0.8.4;
3+
4+
import "../../../access-control/extensions/RBACGroupable.sol";
5+
6+
contract RBACGroupableMock is RBACGroupable {
7+
using ArrayHelper for string;
8+
9+
function __RBACGroupableMock_init() external initializer {
10+
__RBACGroupable_init();
11+
12+
_grantRoles(msg.sender, MASTER_ROLE.asArray());
13+
}
14+
15+
function mockInit() external {
16+
__RBACGroupable_init();
17+
}
18+
}

0 commit comments

Comments
 (0)