diff --git a/contracts/access-control/AccessControlled.sol b/contracts/access-control/AccessControlled.sol index d30a2748..cb68da2d 100644 --- a/contracts/access-control/AccessControlled.sol +++ b/contracts/access-control/AccessControlled.sol @@ -15,9 +15,9 @@ abstract contract AccessControlled is IAccessControlled { IAccessControl private _accessControl; /// @notice Checks if msg.sender has `role`, reverts if not. - /// @param role_ the role to be tested, defined in Roles.sol and set in AccessManager instance. + /// @param role_ the role to be tested, defined in Roles.sol and set in AccessControlSingleton instance. modifier onlyRole(bytes32 role_) { - if (!hasRole(role_, msg.sender)) { + if (!_hasRole(role_, msg.sender)) { revert Errors.MissingRole(role_, msg.sender); } _; @@ -30,7 +30,7 @@ abstract contract AccessControlled is IAccessControlled { emit AccessControlUpdated(accessControl_); } - /// @notice Sets AccessManager instance. Restricted to PROTOCOL_ADMIN_ROLE + /// @notice Sets AccessControlSingleton instance. Restricted to PROTOCOL_ADMIN_ROLE /// @param accessControl_ address of the new instance of AccessControlSingleton. function setAccessControl( address accessControl_ @@ -42,10 +42,10 @@ abstract contract AccessControlled is IAccessControlled { } /// @notice Checks if `account has `role` assigned. - /// @param role_ the role to be tested, defined in Roles.sol and set in AccessManager instance. + /// @param role_ the role to be tested, defined in Roles.sol and set in AccessControlSingleton instance. /// @param account_ the address to be tested for the role. /// @return return true if account has role, false otherwise. - function hasRole( + function _hasRole( bytes32 role_, address account_ ) internal view returns (bool) { diff --git a/contracts/access-control/AccessControlledUpgradeable.sol b/contracts/access-control/AccessControlledUpgradeable.sol index 5be70d73..7c0d95d1 100644 --- a/contracts/access-control/AccessControlledUpgradeable.sol +++ b/contracts/access-control/AccessControlledUpgradeable.sol @@ -22,15 +22,15 @@ abstract contract AccessControlledUpgradeable is UUPSUpgradeable, IAccessControl 0x06c308ca3b780cede1217f5877d0c7fbf50796d93f836cb3b60e6457b0cf03b6; /// @notice Checks if msg.sender has `role`, reverts if not. - /// @param role_ the role to be tested, defined in Roles.sol and set in AccessManager instance. + /// @param role_ the role to be tested, defined in Roles.sol and set in AccessControlSingleton instance. modifier onlyRole(bytes32 role_) { - if (!hasRole(role_, msg.sender)) { + if (!_hasRole(role_, msg.sender)) { revert Errors.MissingRole(role_, msg.sender); } _; } - /// @notice Sets AccessManager instance. Restricted to PROTOCOL_ADMIN_ROLE + /// @notice Sets AccessControlSingleton instance. Restricted to PROTOCOL_ADMIN_ROLE /// @param accessControl_ address of the new instance of AccessControlSingleton. function setAccessControl( address accessControl_ @@ -48,7 +48,7 @@ abstract contract AccessControlledUpgradeable is UUPSUpgradeable, IAccessControl } /// @notice Initializer method, access point to initialize inheritance tree. - /// @param accessControl_ address of AccessManager. + /// @param accessControl_ address of AccessControlSingleton. function __AccessControlledUpgradeable_init( address accessControl_ ) internal initializer { @@ -60,10 +60,10 @@ abstract contract AccessControlledUpgradeable is UUPSUpgradeable, IAccessControl } /// @notice Checks if `account has `role` assigned. - /// @param role_ the role to be tested, defined in Roles.sol and set in AccessManager instance. + /// @param role_ the role to be tested, defined in Roles.sol and set in AccessControlSingleton instance. /// @param account_ the address to be tested for the role. /// @return return true if account has role, false otherwise. - function hasRole( + function _hasRole( bytes32 role_, address account_ ) internal view returns (bool) { diff --git a/contracts/interfaces/access-control/IAccessControlled.sol b/contracts/interfaces/access-control/IAccessControlled.sol index 6a99e99b..b9ef11fc 100644 --- a/contracts/interfaces/access-control/IAccessControlled.sol +++ b/contracts/interfaces/access-control/IAccessControlled.sol @@ -6,4 +6,7 @@ interface IAccessControlled { event AccessControlUpdated(address indexed accessControl); + /// @notice Sets AccessControlSingleton instance. + /// @param accessControl_ address of the new instance of AccessControlSingleton. + function setAccessControl(address accessControl_) external; } diff --git a/contracts/modules/relationships/RelationshipModule.sol b/contracts/modules/relationships/RelationshipModule.sol index 9d26b612..b3457f05 100644 --- a/contracts/modules/relationships/RelationshipModule.sol +++ b/contracts/modules/relationships/RelationshipModule.sol @@ -118,7 +118,7 @@ contract RelationshipModule is BaseModule, IRelationshipModule, AccessControlled /// @param caller_ initiator of the configuration function _verifyConfigCaller(IIPOrg ipOrg_, address caller_) private view { if (address(ipOrg_) == LibRelationship.PROTOCOL_LEVEL_RELATIONSHIP) { - if (!hasRole(AccessControl.RELATIONSHIP_MANAGER_ROLE, caller_)) { + if (!_hasRole(AccessControl.RELATIONSHIP_MANAGER_ROLE, caller_)) { revert Errors.MissingRole(AccessControl.RELATIONSHIP_MANAGER_ROLE, caller_); } } else { diff --git a/test/foundry/access-control/AccessControlSingleton.t.sol b/test/foundry/access-control/AccessControlSingleton.t.sol index 68a24076..a7a37f41 100644 --- a/test/foundry/access-control/AccessControlSingleton.t.sol +++ b/test/foundry/access-control/AccessControlSingleton.t.sol @@ -7,7 +7,6 @@ import { Errors } from "contracts/lib/Errors.sol"; import { AccessControl } from "contracts/lib/AccessControl.sol"; import { AccessControlSingleton } from "contracts/access-control/AccessControlSingleton.sol"; import { AccessControlUpgradeable } from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol"; -import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; contract AccessControlSingletonTest is Test, AccessControlHelper { function setUp() public { @@ -48,15 +47,4 @@ contract AccessControlSingletonTest is Test, AccessControlHelper { vm.expectRevert(_getRoleErrorMessage(address(this), AccessControl.UPGRADER_ROLE)); accessControl.upgradeTo(address(0)); } - - function _getRoleErrorMessage(address sender, bytes32 role) internal pure returns (bytes memory) { - return abi.encodePacked( - "AccessControl: account ", - Strings.toHexString(uint160(sender), 20), - " is missing role ", - Strings.toHexString(uint256(role), 32) - ); - } - - } diff --git a/test/foundry/access-control/AccessControlled.t.sol b/test/foundry/access-control/AccessControlled.t.sol new file mode 100644 index 00000000..f4ae8201 --- /dev/null +++ b/test/foundry/access-control/AccessControlled.t.sol @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: BUSDL-1.1 +pragma solidity ^0.8.13; + +import "forge-std/Test.sol"; +import { AccessControlHelper } from "test/foundry/utils/AccessControlHelper.sol"; +import { Errors } from "contracts/lib/Errors.sol"; +import { AccessControl } from "contracts/lib/AccessControl.sol"; +import { AccessControlled } from "contracts/access-control/AccessControlled.sol"; +import { AccessControlSingleton } from "contracts/access-control/AccessControlSingleton.sol"; +import { MockAccessControlled } from "test/foundry/mocks/MockAccessControlled.sol"; +import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; + +contract AccessControlledTest is Test, AccessControlHelper { + + event AccessControlUpdated(address indexed accessControl); + + MockAccessControlled accessControlled; + + function setUp() public { + _setupAccessControl(); + accessControlled = new MockAccessControlled(address(accessControl)); + } + + function test_AccessControlled_onlyRole() public { + bytes32 role = keccak256("TEST_ROLE"); + _grantRole(vm, role, address(this)); + accessControlled.exposeOnlyRole(role); + } + + function test_AccessControlled_revert_onlyRole() public { + bytes32 role = keccak256("TEST_ROLE"); + vm.expectRevert( + abi.encodeWithSelector( + Errors.MissingRole.selector, + role, + address(this) + ) + ); + accessControlled.exposeOnlyRole(role); + } + + function test_AccessControlled_setAccessControl() public { + AccessControlSingleton ac2 = new AccessControlSingleton(); + vm.expectEmit(true, true, true, true); + emit AccessControlUpdated(address(ac2)); + vm.prank(admin); + accessControlled.setAccessControl(address(ac2)); + } + + function test_AccessControlled_revert_setAccessControlNotProtocolAdmin() public { + AccessControlSingleton ac2 = new AccessControlSingleton(); + vm.expectRevert( + abi.encodeWithSelector( + Errors.MissingRole.selector, + AccessControl.PROTOCOL_ADMIN_ROLE, + address(this) + ) + ); + accessControlled.setAccessControl(address(ac2)); + } + +} diff --git a/test/foundry/access-control/AccessControlledUpgradeable.t.sol b/test/foundry/access-control/AccessControlledUpgradeable.t.sol new file mode 100644 index 00000000..53e368bc --- /dev/null +++ b/test/foundry/access-control/AccessControlledUpgradeable.t.sol @@ -0,0 +1,70 @@ +// SPDX-License-Identifier: BUSDL-1.1 +pragma solidity ^0.8.13; + +import "forge-std/Test.sol"; +import { AccessControlHelper } from "test/foundry/utils/AccessControlHelper.sol"; +import { Errors } from "contracts/lib/Errors.sol"; +import { AccessControl } from "contracts/lib/AccessControl.sol"; +import { AccessControlledUpgradeable } from "contracts/access-control/AccessControlledUpgradeable.sol"; +import { AccessControlSingleton } from "contracts/access-control/AccessControlSingleton.sol"; +import { MockAccessControlledUpgradeable } from "test/foundry/mocks/MockAccessControlledUpgradeable.sol"; +import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; + +contract AccessControlledUpgradeableTest is Test, AccessControlHelper { + + event AccessControlUpdated(address indexed accessControl); + + MockAccessControlledUpgradeable accessControlled; + + function setUp() public { + _setupAccessControl(); + accessControlled = MockAccessControlledUpgradeable( + _deployUUPSProxy( + address(new MockAccessControlledUpgradeable()), + abi.encodeWithSelector( + bytes4(keccak256(bytes("initialize(address)"))), + address(accessControl) + ) + ) + ); + } + + function test_AccessControlled_onlyRole() public { + bytes32 role = keccak256("TEST_ROLE"); + _grantRole(vm, role, address(this)); + accessControlled.exposeOnlyRole(role); + } + + function test_AccessControlled_revert_onlyRole() public { + bytes32 role = keccak256("TEST_ROLE"); + vm.expectRevert( + abi.encodeWithSelector( + Errors.MissingRole.selector, + role, + address(this) + ) + ); + accessControlled.exposeOnlyRole(role); + } + + function test_AccessControlled_setAccessControl() public { + AccessControlSingleton ac2 = new AccessControlSingleton(); + vm.expectEmit(true, true, true, true); + emit AccessControlUpdated(address(ac2)); + vm.prank(admin); + accessControlled.setAccessControl(address(ac2)); + } + + function test_AccessControlled_revert_setAccessControlNotProtocolAdmin() public { + AccessControlSingleton ac2 = new AccessControlSingleton(); + vm.expectRevert( + abi.encodeWithSelector( + Errors.MissingRole.selector, + AccessControl.PROTOCOL_ADMIN_ROLE, + address(this) + ) + ); + accessControlled.setAccessControl(address(ac2)); + } + +} diff --git a/test/foundry/mocks/MockAccessControlled.sol b/test/foundry/mocks/MockAccessControlled.sol new file mode 100644 index 00000000..a30fba86 --- /dev/null +++ b/test/foundry/mocks/MockAccessControlled.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: BUSDL-1.1 +pragma solidity ^0.8.13; + +import { AccessControlled } from "contracts/access-control/AccessControlled.sol"; + +contract MockAccessControlled is AccessControlled { + constructor(address accessControl) AccessControlled(accessControl) {} + + function exposeOnlyRole(bytes32 role) public onlyRole(role) {} +} + diff --git a/test/foundry/mocks/MockAccessControlledUpgradeable.sol b/test/foundry/mocks/MockAccessControlledUpgradeable.sol new file mode 100644 index 00000000..770d76fa --- /dev/null +++ b/test/foundry/mocks/MockAccessControlledUpgradeable.sol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: BUSDL-1.1 +pragma solidity ^0.8.13; + +import { AccessControlledUpgradeable } from "contracts/access-control/AccessControlledUpgradeable.sol"; + +contract MockAccessControlledUpgradeable is AccessControlledUpgradeable { + + function initialize(address accessControl) public initializer { + __AccessControlledUpgradeable_init(accessControl); + } + + function exposeOnlyRole(bytes32 role) public onlyRole(role) {} + + function _authorizeUpgrade(address newImplementation) internal virtual override {} +} diff --git a/test/foundry/utils/AccessControlHelper.sol b/test/foundry/utils/AccessControlHelper.sol index 818a4655..32a04427 100644 --- a/test/foundry/utils/AccessControlHelper.sol +++ b/test/foundry/utils/AccessControlHelper.sol @@ -1,16 +1,16 @@ // SPDX-License-Identifier: BUSDL-1.1 pragma solidity ^0.8.19; -import 'test/foundry/utils/ProxyHelper.sol'; +import "test/foundry/utils/ProxyHelper.sol"; import { AccessControl } from "contracts/lib/AccessControl.sol"; import { AccessControlSingleton } from "contracts/access-control/AccessControlSingleton.sol"; import { Vm } from "forge-std/Test.sol"; import "forge-std/console.sol"; +import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; /// @title AccessControlHelper /// @notice Helper contract to setup AccessControlSingleton and grant roles contract AccessControlHelper is ProxyHelper { - AccessControlSingleton accessControl; address admin = address(123); @@ -18,12 +18,15 @@ contract AccessControlHelper is ProxyHelper { function _setupAccessControl() internal { // Create Access Control - address accessControlSingletonImpl = address(new AccessControlSingleton()); + address accessControlSingletonImpl = address( + new AccessControlSingleton() + ); accessControl = AccessControlSingleton( _deployUUPSProxy( accessControlSingletonImpl, abi.encodeWithSelector( - bytes4(keccak256(bytes("initialize(address)"))), admin + bytes4(keccak256(bytes("initialize(address)"))), + admin ) ) ); @@ -34,4 +37,16 @@ contract AccessControlHelper is ProxyHelper { accessControl.grantRole(role, account); } + function _getRoleErrorMessage( + address sender, + bytes32 role + ) internal pure returns (bytes memory) { + return + abi.encodePacked( + "AccessControl: account ", + Strings.toHexString(uint160(sender), 20), + " is missing role ", + Strings.toHexString(uint256(role), 32) + ); + } }