From 663edd2ba8b2f809dff2bb8283787d4c655bb651 Mon Sep 17 00:00:00 2001 From: shaito Date: Wed, 25 Sep 2024 11:35:25 +0200 Subject: [PATCH] feat: permit access control --- foundry.toml | 2 +- package.json | 4 +- solidity/contracts/AccessController.sol | 22 +++++-- solidity/contracts/Oracle.sol | 20 ++++-- solidity/contracts/OracleTypehash.sol | 8 +++ .../contracts/PermitAccessControlModule.sol | 62 +++++++++++++++++++ .../accessControl/IAccessControlModule.sol | 8 ++- yarn.lock | 5 ++ 8 files changed, 118 insertions(+), 13 deletions(-) create mode 100644 solidity/contracts/OracleTypehash.sol create mode 100644 solidity/contracts/PermitAccessControlModule.sol diff --git a/foundry.toml b/foundry.toml index 9277e53..cd2980e 100644 --- a/foundry.toml +++ b/foundry.toml @@ -9,7 +9,7 @@ multiline_func_header = 'params_first' sort_imports = true [profile.default] -solc_version = '0.8.19' +solc_version = '0.8.20' src = 'solidity' test = 'solidity/test' out = 'out' diff --git a/package.json b/package.json index 74cc75c..52d454f 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "devDependencies": { "@commitlint/cli": "17.0.3", "@commitlint/config-conventional": "17.0.3", + "@openzeppelin/contracts": "^5.0.2", "dotenv-cli": "7.2.1", "ds-test": "https://github.com/dapphub/ds-test.git#e282159d5170298eb2455a6c05280ab5a73a4ef0", "forge-std": "https://github.com/foundry-rs/forge-std.git#v1.7.3", @@ -48,5 +49,6 @@ "solhint-plugin-defi-wonderland": "1.1.2", "sort-package-json": "2.4.1", "standard-version": "9.5.0" - } + }, + "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e" } diff --git a/solidity/contracts/AccessController.sol b/solidity/contracts/AccessController.sol index 09f3e4a..69beae7 100644 --- a/solidity/contracts/AccessController.sol +++ b/solidity/contracts/AccessController.sol @@ -14,17 +14,29 @@ abstract contract AccessController { bytes data; } + error AccessController_NoAccess(); + /** * @notice Modifier to check if the caller has access to the user * @param _accessControlModule The access control module - * @param _caller The caller * @param _accessControl The access control struct */ - modifier hasAccess(IAccessControlModule _accessControlModule, address _caller, AccessControl memory _accessControl) { - bool _hasAccess = _caller == _accessControl.user + modifier hasAccess( + address _accessControlModule, + bytes32 _typehash, + bytes memory _params, + AccessControl memory _accessControl + ) { + bool _hasAccess = msg.sender == _accessControl.user || ( - address(_accessControlModule) != address(0) - && _accessControlModule.hasAccess(_caller, _accessControl.user, _accessControl.data) + _accessControlModule != address(0) + && IAccessControlModule(_accessControlModule).hasAccess({ + _caller: msg.sender, + _user: _accessControl.user, + _typehash: _typehash, + _params: _params, + _data: _accessControl.data + }) ); if (!_hasAccess) revert AccessController_NoAccess(); _; diff --git a/solidity/contracts/Oracle.sol b/solidity/contracts/Oracle.sol index 00b1d46..42ddf63 100644 --- a/solidity/contracts/Oracle.sol +++ b/solidity/contracts/Oracle.sol @@ -13,7 +13,9 @@ import {ValidatorLib} from '../libraries/ValidatorLib.sol'; import {AccessController} from './AccessController.sol'; -contract Oracle is IOracle, AccessController { +import {OracleTypehash} from './OracleTypehash.sol'; + +contract Oracle is IOracle, AccessController, OracleTypehash { using ValidatorLib for *; /// @inheritdoc IOracle @@ -114,7 +116,11 @@ contract Oracle is IOracle, AccessController { Request calldata _request, Response calldata _response, AccessControl memory _accessControl - ) external hasAccess(_request.accessControlModule, msg.sender, _accessControl) returns (bytes32 _responseId) { + ) + external + hasAccess(_request.accessControlModule, PROPOSE_TYPEHASH, abi.encode(_request, _response), _accessControl) + returns (bytes32 _responseId) + { _responseId = ValidatorLib._validateResponse(_request, _response); bytes32 _requestId = _response.requestId; @@ -210,7 +216,7 @@ contract Oracle is IOracle, AccessController { IDisputeModule(_request.disputeModule).onDisputeStatusChange(_disputeId, _request, _response, _dispute); // TODO: Let's remove the msg.sender from this event and we can remove the access control - emit DisputeEscalated(msg.sender, _disputeId, block.number); + emit DisputeEscalated(msg.sender, _disputeId, block.number); if (address(_request.resolutionModule) != address(0)) { // Initiate the resolution @@ -312,7 +318,9 @@ contract Oracle is IOracle, AccessController { } /// @inheritdoc IOracle - function getResponseIds(bytes32 _requestId) public view returns (bytes32[] memory _ids) { + function getResponseIds( + bytes32 _requestId + ) public view returns (bytes32[] memory _ids) { bytes memory _responses = _responseIds[_requestId]; uint256 _length = _responses.length / 32; @@ -370,7 +378,9 @@ contract Oracle is IOracle, AccessController { * @param _request The request to be finalized * @return _requestId The id of the finalized request */ - function _finalizeWithoutResponse(IOracle.Request calldata _request) internal view returns (bytes32 _requestId) { + function _finalizeWithoutResponse( + IOracle.Request calldata _request + ) internal view returns (bytes32 _requestId) { _requestId = ValidatorLib._getId(_request); if (requestCreatedAt[_requestId] == 0) { diff --git a/solidity/contracts/OracleTypehash.sol b/solidity/contracts/OracleTypehash.sol new file mode 100644 index 0000000..38838b5 --- /dev/null +++ b/solidity/contracts/OracleTypehash.sol @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +contract OracleTypehash { + bytes32 internal constant PROPOSE_TYPEHASH = keccak256('ProposeResponse(Request _request, Response _response)'); + bytes32 internal constant DISPUTE_TYPEHASH = + keccak256('DisputeResponse(Request _request, Response _response, Dispute _dispute,)'); +} diff --git a/solidity/contracts/PermitAccessControlModule.sol b/solidity/contracts/PermitAccessControlModule.sol new file mode 100644 index 0000000..2ce66c2 --- /dev/null +++ b/solidity/contracts/PermitAccessControlModule.sol @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IAccessControlModule} from '../interfaces/modules/accessControl/IAccessControlModule.sol'; +import {Nonces} from '@openzeppelin/contracts/utils/Nonces.sol'; + +import {ECDSA} from '@openzeppelin/contracts/utils/cryptography/ECDSA.sol'; +import {EIP712} from '@openzeppelin/contracts/utils/cryptography/EIP712.sol'; + +contract PermitAccessControl is Nonces, EIP712 { + /** + * @notice The access control struct + * @param user The address of the user + * @param data The data for access control validation + */ + struct AccessControlData { + uint256 nonce; + uint256 deadline; + uint8 v; + bytes32 r; + bytes32 s; + } + + error ERC2612ExpiredSignature(uint256 deadline); + error ERC2612InvalidSigner(address signer, address owner); + + /** + * @dev Initializes the {EIP712} domain separator using the `name` parameter, and setting `version` to `"1"`. + * + * It's a good idea to use the same `name` that is defined as the ERC-20 token name. + */ + constructor( + string memory name + ) EIP712(name, '1') {} + + function hasAccess( + address, + address _user, + bytes32 _signature, + bytes memory _params, + bytes calldata _data + ) external returns (bool _hasAccess) { + AccessControlData memory _permit = abi.decode(_data, (AccessControlData)); + // I don't think you care about validating the _caller + // You do care for repeatability, so a nonce is 100% necessary + // You care about expiration, so a deadline is 100% necessary + // You care abxout the function and the parameters that were approved + if (block.timestamp > _permit.deadline) { + revert ERC2612ExpiredSignature(_permit.deadline); + } + // signature, params (removing the last parameter for the access control), nonce, deadline + bytes32 structHash = keccak256(abi.encode(_signature, _params, _useNonce(_user), _permit.deadline)); + + bytes32 hash = _hashTypedDataV4(structHash); + + address signer = ECDSA.recover(hash, _permit.v, _permit.r, _permit.s); + + if (signer != _user) { + revert ERC2612InvalidSigner(signer, _user); + } + } +} diff --git a/solidity/interfaces/modules/accessControl/IAccessControlModule.sol b/solidity/interfaces/modules/accessControl/IAccessControlModule.sol index c2c2084..16dd503 100644 --- a/solidity/interfaces/modules/accessControl/IAccessControlModule.sol +++ b/solidity/interfaces/modules/accessControl/IAccessControlModule.sol @@ -8,5 +8,11 @@ import {IModule} from '../../IModule.sol'; * @notice Common interface for all response modules */ interface IAccessControlModule is IModule { - function hasAccess(address _caller, address _user, bytes calldata _data) external view returns (bool _hasAccess); + function hasAccess( + address _caller, + address _user, + bytes32 _typehash, + bytes memory _params, + bytes calldata _data + ) external returns (bool _hasAccess); } diff --git a/yarn.lock b/yarn.lock index 2c7a91b..fc66992 100644 --- a/yarn.lock +++ b/yarn.lock @@ -227,6 +227,11 @@ "@nodelib/fs.scandir" "2.1.5" fastq "^1.6.0" +"@openzeppelin/contracts@^5.0.2": + version "5.0.2" + resolved "https://registry.yarnpkg.com/@openzeppelin/contracts/-/contracts-5.0.2.tgz#b1d03075e49290d06570b2fd42154d76c2a5d210" + integrity sha512-ytPc6eLGcHHnapAZ9S+5qsdomhjo6QBHTDRRBFfTxXIpsicMhVPouPgmUPebZZZGX7vt9USA+Z+0M0dSVtSUEA== + "@solidity-parser/parser@^0.14.1": version "0.14.5" resolved "https://registry.yarnpkg.com/@solidity-parser/parser/-/parser-0.14.5.tgz#87bc3cc7b068e08195c219c91cd8ddff5ef1a804"