Extend onlyAuthorized to support extra functions in AccessManager (#5014)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
This commit is contained in:
Ernesto García
2024-05-27 03:09:42 -06:00
committed by GitHub
parent b64c90202a
commit 52e0e3e783
7 changed files with 177 additions and 34 deletions

View File

@ -412,9 +412,6 @@ contract AccessManager is Context, Multicall, IAccessManager {
* Emits a {TargetClosed} event.
*/
function _setTargetClosed(address target, bool closed) internal virtual {
if (target == address(this)) {
revert AccessManagerLockedAccount(target);
}
_targets[target].closed = closed;
emit TargetClosed(target, closed);
}
@ -586,7 +583,9 @@ contract AccessManager is Context, Multicall, IAccessManager {
// ================================================= ADMIN LOGIC ==================================================
/**
* @dev Check if the current call is authorized according to admin logic.
* @dev Check if the current call is authorized according to admin and roles logic.
*
* WARNING: Carefully review the considerations of {AccessManaged-restricted} since they apply to this modifier.
*/
function _checkAuthorized() private {
address caller = _msgSender();
@ -611,7 +610,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
*/
function _getAdminRestrictions(
bytes calldata data
) private view returns (bool restricted, uint64 roleAdminId, uint32 executionDelay) {
) private view returns (bool adminRestricted, uint64 roleAdminId, uint32 executionDelay) {
if (data.length < 4) {
return (false, 0, 0);
}
@ -648,7 +647,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
return (true, getRoleAdmin(roleId), 0);
}
return (false, 0, 0);
return (false, getTargetFunctionRole(address(this), selector), 0);
}
// =================================================== HELPERS ====================================================
@ -673,7 +672,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
}
/**
* @dev A version of {canCall} that checks for admin restrictions in this contract.
* @dev A version of {canCall} that checks for restrictions in this contract.
*/
function _canCallSelf(address caller, bytes calldata data) private view returns (bool immediate, uint32 delay) {
if (data.length < 4) {
@ -686,8 +685,10 @@ contract AccessManager is Context, Multicall, IAccessManager {
return (_isExecuting(address(this), _checkSelector(data)), 0);
}
(bool enabled, uint64 roleId, uint32 operationDelay) = _getAdminRestrictions(data);
if (!enabled) {
(bool adminRestricted, uint64 roleId, uint32 operationDelay) = _getAdminRestrictions(data);
// isTragetClosed apply to non-admin-restricted function
if (!adminRestricted && isTargetClosed(address(this))) {
return (false, 0);
}

View File

@ -82,7 +82,6 @@ interface IAccessManager {
error AccessManagerNotScheduled(bytes32 operationId);
error AccessManagerNotReady(bytes32 operationId);
error AccessManagerExpired(bytes32 operationId);
error AccessManagerLockedAccount(address account);
error AccessManagerLockedRole(uint64 roleId);
error AccessManagerBadConfirmation();
error AccessManagerUnauthorizedAccount(address msgsender, uint64 roleId);
@ -108,7 +107,7 @@ interface IAccessManager {
* is backward compatible. Some contracts may thus ignore the second return argument. In that case they will fail
* to identify the indirect workflow, and will consider calls that require a delay to be forbidden.
*
* NOTE: This function does not report the permissions of this manager itself. These are defined by the
* NOTE: This function does not report the permissions of the admin functions in the manager itself. These are defined by the
* {AccessManager} documentation.
*/
function canCall(
@ -134,6 +133,8 @@ interface IAccessManager {
/**
* @dev Get whether the contract is closed disabling any access. Otherwise role permissions are applied.
*
* NOTE: When the manager itself is closed, admin functions are still accessible to avoid locking the contract.
*/
function isTargetClosed(address target) external view returns (bool);
@ -308,6 +309,8 @@ interface IAccessManager {
/**
* @dev Set the closed flag for a contract.
*
* Closing the manager itself won't disable access to admin methods to avoid locking the contract.
*
* Requirements:
*
* - the caller must be a global admin