Skip to content

Commit

Permalink
refactor: permission manager
Browse files Browse the repository at this point in the history
  • Loading branch information
@heueristik
heueristik committed Sep 4, 2023
1 parent cde6547 commit 3c3aad4
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 113 deletions.
61 changes: 8 additions & 53 deletions packages/contracts/src/core/dao/DAO.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ contract DAO is
bytes32 _permissionId,
bytes memory _data
) external view override returns (bool) {
return isGranted(_where, _who, _permissionId, _data);
return isGranted({_where: _where, _who: _who, _permissionId: _permissionId, _data: _data});
}

/// @inheritdoc IDAO
Expand Down Expand Up @@ -328,11 +328,6 @@ contract DAO is
revert FunctionRemoved();
}

bytes4 private constant VALID_ERC1271_SIGNATURE = 0x1626ba7e; // `type(IERC1271).interfaceId` = bytes4(keccak256("isValidSignature(bytes32,bytes)")`
bytes4 private constant INVALID_ERC1271_SIGNATURE = 0xffffffff; // `bytes4(uint32(type(uint32).max-1))`

error SignatureError();

/// @inheritdoc IDAO
/// @dev Relays the validation logic determining who is allowed to sign on behalf of the DAO to its permission manager.
/// Caller specific bypassing can be set direct granting (i.e., `grant({_where: dao, _who: specificErc1271Caller, _permissionId: VALIDATE_SIGNATURE_PERMISSION_ID})`).
Expand All @@ -342,57 +337,17 @@ contract DAO is
bytes32 _hash,
bytes memory _signature
) external view override(IDAO, IERC1271) returns (bytes4) {
address accessFlagOrCondition = permissionsHashed[
permissionHash(address(this), msg.sender, VALIDATE_SIGNATURE_PERMISSION_ID)
];

if (accessFlagOrCondition == UNSET_FLAG) return INVALID_ERC1271_SIGNATURE;
// Caller-specific bypassing
if (accessFlagOrCondition == ALLOW_FLAG) return VALID_ERC1271_SIGNATURE;

bytes memory data = abi.encode(_hash, _signature);

// Caller-specific condition
if (
_isSignatureConditionValid({
_condition: accessFlagOrCondition,
_caller: msg.sender,
_data: data
})
) {
return VALID_ERC1271_SIGNATURE;
}

// Generic condition
if (
_isSignatureConditionValid({
_condition: accessFlagOrCondition,
_caller: ANY_ADDR,
_data: data
})
) {
return VALID_ERC1271_SIGNATURE;
}

return INVALID_ERC1271_SIGNATURE;
}

function _isSignatureConditionValid(
address _condition,
address _caller,
bytes memory _data
) internal view returns (bool) {
try
IPermissionCondition(_condition).isGranted({
isGranted({
_where: address(this),
_who: _caller,
_who: msg.sender,
_permissionId: VALIDATE_SIGNATURE_PERMISSION_ID,
_data: _data
_data: abi.encode(_hash, _signature)
})
returns (bool valid) {
if (valid) return true;
} catch {}
return false;
) {
return 0x1626ba7e; // `type(IERC1271).interfaceId` = bytes4(keccak256("isValidSignature(bytes32,bytes)")`
}
return 0xffffffff; // `bytes4(uint32(type(uint32).max-1))`
}

/// @notice Emits the `NativeTokenDeposited` event to track native token deposits that weren't made via the deposit method.
Expand Down
197 changes: 138 additions & 59 deletions packages/contracts/src/core/permission/PermissionManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ abstract contract PermissionManager is Initializable {
/// @dev The initial owner is granted the `ROOT_PERMISSION_ID` permission.
/// @param _initialOwner The initial owner of the permission manager.
function __PermissionManager_init(address _initialOwner) internal onlyInitializing {
_initializePermissionManager(_initialOwner);
_initializePermissionManager({_initialOwner: _initialOwner});
}

/// @notice Grants permission to an address to call methods in a contract guarded by an auth modifier with the specified permission identifier.
Expand All @@ -119,7 +119,7 @@ abstract contract PermissionManager is Initializable {
address _who,
bytes32 _permissionId
) external virtual auth(ROOT_PERMISSION_ID) {
_grant(_where, _who, _permissionId);
_grant({_where: _where, _who: _who, _permissionId: _permissionId});
}

/// @notice Grants permission to an address to call methods in a target contract guarded by an auth modifier with the specified permission identifier if the referenced condition permits it.
Expand All @@ -135,7 +135,12 @@ abstract contract PermissionManager is Initializable {
bytes32 _permissionId,
IPermissionCondition _condition
) external virtual auth(ROOT_PERMISSION_ID) {
_grantWithCondition(_where, _who, _permissionId, _condition);
_grantWithCondition({
_where: _where,
_who: _who,
_permissionId: _permissionId,
_condition: _condition
});
}

/// @notice Revokes permission from an address to call methods in a target contract guarded by an auth modifier with the specified permission identifier.
Expand All @@ -149,7 +154,7 @@ abstract contract PermissionManager is Initializable {
address _who,
bytes32 _permissionId
) external virtual auth(ROOT_PERMISSION_ID) {
_revoke(_where, _who, _permissionId);
_revoke({_where: _where, _who: _who, _permissionId: _permissionId});
}

/// @notice Applies an array of permission operations on a single target contracts `_where`.
Expand All @@ -163,9 +168,9 @@ abstract contract PermissionManager is Initializable {
PermissionLib.SingleTargetPermission memory item = items[i];

if (item.operation == PermissionLib.Operation.Grant) {
_grant(_where, item.who, item.permissionId);
_grant({_where: _where, _who: item.who, _permissionId: item.permissionId});
} else if (item.operation == PermissionLib.Operation.Revoke) {
_revoke(_where, item.who, item.permissionId);
_revoke({_where: _where, _who: item.who, _permissionId: item.permissionId});
} else if (item.operation == PermissionLib.Operation.GrantWithCondition) {
revert GrantWithConditionNotSupported();
}
Expand All @@ -185,16 +190,16 @@ abstract contract PermissionManager is Initializable {
PermissionLib.MultiTargetPermission memory item = _items[i];

if (item.operation == PermissionLib.Operation.Grant) {
_grant(item.where, item.who, item.permissionId);
_grant({_where: item.where, _who: item.who, _permissionId: item.permissionId});
} else if (item.operation == PermissionLib.Operation.Revoke) {
_revoke(item.where, item.who, item.permissionId);
_revoke({_where: item.where, _who: item.who, _permissionId: item.permissionId});
} else if (item.operation == PermissionLib.Operation.GrantWithCondition) {
_grantWithCondition(
item.where,
item.who,
item.permissionId,
IPermissionCondition(item.condition)
);
_grantWithCondition({
_where: item.where,
_who: item.who,
_permissionId: item.permissionId,
_condition: IPermissionCondition(item.condition)
});
}

unchecked {
Expand All @@ -215,16 +220,100 @@ abstract contract PermissionManager is Initializable {
bytes32 _permissionId,
bytes memory _data
) public view virtual returns (bool) {
return
_isGranted(_where, _who, _permissionId, _data) || // check if `_who` has permission for `_permissionId` on `_where`
_isGranted(_where, ANY_ADDR, _permissionId, _data) || // check if anyone has permission for `_permissionId` on `_where`
_isGranted(ANY_ADDR, _who, _permissionId, _data); // check if `_who` has permission for `_permissionI` on any contract
// Specific caller and target
{
address specificPermissionOrCondition = permissionsHashed[
permissionHash({_where: _where, _who: _who, _permissionId: _permissionId})
];

if (specificPermissionOrCondition == ALLOW_FLAG) return true;

if (specificPermissionOrCondition != UNSET_FLAG) {
if (
_checkCondition({
_condition: specificPermissionOrCondition,
_where: _where,
_who: _who,
_permissionId: _permissionId,
_data: _data
})
) return true;
}
}

// Generic caller
{
address genericCallerCondition = permissionsHashed[
permissionHash({_where: _where, _who: ANY_ADDR, _permissionId: _permissionId})
];

if (genericCallerCondition != UNSET_FLAG) {
if (
_checkCondition({
_condition: genericCallerCondition,
_where: _where,
_who: _who,
_permissionId: _permissionId,
_data: _data
})
) return true;
}
}
// Generic target
{
address genericTargetCondition = permissionsHashed[
permissionHash({_where: ANY_ADDR, _who: _who, _permissionId: _permissionId})
];
if (genericTargetCondition != UNSET_FLAG) {
if (
_checkCondition({
_condition: genericTargetCondition,
_where: _where,
_who: _who,
_permissionId: _permissionId,
_data: _data
})
) return true;
}
}

return false;
}

/// @notice Checks an condition contract by doing an external call.
/// @param _condition The condition contract that is asked.
/// @param _where The address of the target contract for which `_who` receives permission.
/// @param _who The address (EOA or contract) owning the permission.
/// @param _permissionId The permission identifier.
/// @param _data The optional data passed to the `PermissionCondition` registered.
/// @return Returns true if `_who` has the permissions on the contract via the specified permissionId identifier.
function _checkCondition(
address _condition,
address _where,
address _who,
bytes32 _permissionId,
bytes memory _data
) internal view virtual returns (bool) {
if (_condition == UNSET_FLAG) return false;

// try-catch to skip failures
try
IPermissionCondition(_condition).isGranted({
_where: _where,
_who: _who,
_permissionId: _permissionId,
_data: _data
})
returns (bool result) {
if (result) return true;
} catch {}
return false;
}

/// @notice Grants the `ROOT_PERMISSION_ID` permission to the initial owner during initialization of the permission manager.
/// @param _initialOwner The initial owner of the permission manager.
function _initializePermissionManager(address _initialOwner) internal {
_grant(address(this), _initialOwner, ROOT_PERMISSION_ID);
_grant({_where: address(this), _who: _initialOwner, _permissionId: ROOT_PERMISSION_ID});
}

/// @notice This method is used in the external `grant` method of the permission manager.
Expand All @@ -237,15 +326,25 @@ abstract contract PermissionManager is Initializable {
revert PermissionsForAnyAddressDisallowed();
}

bytes32 permHash = permissionHash(_where, _who, _permissionId);
bytes32 permHash = permissionHash({
_where: _where,
_who: _who,
_permissionId: _permissionId
});

address currentFlag = permissionsHashed[permHash];

// Means permHash is not currently set.
if (currentFlag == UNSET_FLAG) {
permissionsHashed[permHash] = ALLOW_FLAG;

emit Granted(_permissionId, msg.sender, _where, _who, ALLOW_FLAG);
emit Granted({
permissionId: _permissionId,
here: msg.sender,
where: _where,
who: _who,
condition: ALLOW_FLAG
});
}
}

Expand Down Expand Up @@ -288,15 +387,25 @@ abstract contract PermissionManager is Initializable {
}
}

bytes32 permHash = permissionHash(_where, _who, _permissionId);
bytes32 permHash = permissionHash({
_where: _where,
_who: _who,
_permissionId: _permissionId
});

address currentCondition = permissionsHashed[permHash];

// Means permHash is not currently set.
if (currentCondition == UNSET_FLAG) {
permissionsHashed[permHash] = conditionAddr;

emit Granted(_permissionId, msg.sender, _where, _who, conditionAddr);
emit Granted({
permissionId: _permissionId,
here: msg.sender,
where: _where,
who: _who,
condition: conditionAddr
});
} else if (currentCondition != conditionAddr) {
// Revert if `permHash` is already granted, but uses a different condition.
// If we don't revert, we either should:
Expand All @@ -319,48 +428,18 @@ abstract contract PermissionManager is Initializable {
/// @param _permissionId The permission identifier.
/// @dev Note, that revoking permissions with `_who` or `_where` equal to `ANY_ADDR` does not revoke other permissions with specific `_who` and `_where` addresses that might have been granted in parallel.
function _revoke(address _where, address _who, bytes32 _permissionId) internal virtual {
bytes32 permHash = permissionHash(_where, _who, _permissionId);
bytes32 permHash = permissionHash({
_where: _where,
_who: _who,
_permissionId: _permissionId
});
if (permissionsHashed[permHash] != UNSET_FLAG) {
permissionsHashed[permHash] = UNSET_FLAG;

emit Revoked(_permissionId, msg.sender, _where, _who);
emit Revoked({permissionId: _permissionId, here: msg.sender, where: _where, who: _who});
}
}

/// @notice Checks if a caller is granted permissions on a target contract via a permission identifier and redirects the approval to a `PermissionCondition` if this was specified in the setup.
/// @param _where The address of the target contract for which `_who` receives permission.
/// @param _who The address (EOA or contract) owning the permission.
/// @param _permissionId The permission identifier.
/// @param _data The optional data passed to the `PermissionCondition` registered.
/// @return Returns true if `_who` has the permissions on the contract via the specified permissionId identifier.
function _isGranted(
address _where,
address _who,
bytes32 _permissionId,
bytes memory _data
) internal view virtual returns (bool) {
address accessFlagOrCondition = permissionsHashed[
permissionHash(_where, _who, _permissionId)
];

if (accessFlagOrCondition == UNSET_FLAG) return false;
if (accessFlagOrCondition == ALLOW_FLAG) return true;

// Since it's not a flag, assume it's a PermissionCondition and try-catch to skip failures
try
IPermissionCondition(accessFlagOrCondition).isGranted(
_where,
_who,
_permissionId,
_data
)
returns (bool allowed) {
if (allowed) return true;
} catch {}

return false;
}

/// @notice A private function to be used to check permissions on the permission manager contract (`address(this)`) itself.
/// @param _permissionId The permission identifier required to call the method this modifier is applied to.
function _auth(bytes32 _permissionId) internal view virtual {
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts/src/test/permission/PermissionManagerTest.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ contract PermissionManagerTest is PermissionManager {
bytes32 _permissionId,
bytes memory _data
) public view returns (bool) {
return _isGranted(_where, _who, _permissionId, _data);
return isGranted(_where, _who, _permissionId, _data);
}

function isPermissionRestrictedForAnyAddr(
Expand Down
1 change: 1 addition & 0 deletions packages/contracts/test/core/dao/dao.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1330,6 +1330,7 @@ describe('DAO', function () {
expect(
await dao.connect(caller).isValidSignature(hash, signature)
).to.equal(VALID_ERC1271_SIGNATURE);

expect(
await dao.connect(otherCaller).isValidSignature(hash, signature)
).to.equal(VALID_ERC1271_SIGNATURE);
Expand Down

0 comments on commit 3c3aad4

Please sign in to comment.