feat: add _msgSenderOnlyEVCAccountOwner

Author: kasperpawlowski

src/utils/EVCUtil.sol

@@ -57,7 +57,7 @@ abstract contract EVCUtil {
     /// @dev This modifier can be used on access controlled functions to prevent non-standard authentication paths on
     /// the EVC.
     modifier onlyEVCAccount() virtual {
-        _onlyEVCAccount(false);
+        _authenticateCallerWithStandardContextState(false);
         _;
     }
 
@@ -71,7 +71,7 @@ abstract contract EVCUtil {
     /// @dev This modifier can be used on access controlled functions to prevent non-standard authentication paths on
     /// the EVC.
     modifier onlyEVCAccountOwner() virtual {
-        _onlyEVCAccount(true);
+        _authenticateCallerWithStandardContextState(true);
         _;
     }
 
@@ -133,6 +133,13 @@ abstract contract EVCUtil {
         return sender;
     }
 
+    /// @notice Retrieves the message sender, ensuring it's the EVC account owner and that the execution context is in a
+    /// standard state (not operator authenticated, not control collateral in progress, not checks in progress).
+    /// @return The address of the message sender.
+    function _msgSenderOnlyEVCAccountOwner() internal view returns (address) {
+        return _authenticateCallerWithStandardContextState(true);
+    }
+
     /// @notice Calls the current external function through the EVC.
     /// @dev This function is used to route the current call through the EVC if it's not already coming from the EVC. It
     /// makes the EVC set the execution context and call back this contract with unchanged calldata. msg.sender is used
@@ -177,22 +184,28 @@ abstract contract EVCUtil {
     /// onlyAccountOwner is true and the owner was already registered on the EVC, it verifies that the onBehalfOfAccount
     /// is the owner. If onlyAccountOwner is false, it allows any EVC account of the owner to call the function.
     /// @param onlyAccountOwner If true, only allows the account owner; if false, allows any EVC account of the owner
-    function _onlyEVCAccount(bool onlyAccountOwner) internal view {
+    /// @return The address of the message sender.
+    function _authenticateCallerWithStandardContextState(bool onlyAccountOwner) internal view returns (address) {
         if (msg.sender == address(evc)) {
             EC ec = EC.wrap(evc.getRawExecutionContext());
 
             if (ec.isOperatorAuthenticated() || ec.isControlCollateralInProgress() || ec.areChecksInProgress()) {
                 revert NotAuthorized();
             }
 
+            address onBehalfOfAccount = ec.getOnBehalfOfAccount();
+
             if (onlyAccountOwner) {
-                address onBehalfOfAccount = ec.getOnBehalfOfAccount();
                 address owner = evc.getAccountOwner(onBehalfOfAccount);
 
                 if (owner != address(0) && owner != onBehalfOfAccount) {
                     revert NotAuthorized();
                 }
             }
+
+            return onBehalfOfAccount;
         }
+
+        return msg.sender;
     }
 }

test/unit/EVCUtil/EVCUtil.t.sol

@@ -57,6 +57,10 @@ contract EVCClient is EVCUtil {
         return _msgSenderForBorrow();
     }
 
+    function msgSenderOnlyEVCAccountOwner() external view returns (address) {
+        return _msgSenderOnlyEVCAccountOwner();
+    }
+
     fallback(bytes calldata) external returns (bytes memory) {
         return abi.encode(IVault.checkAccountStatus.selector);
     }
@@ -191,7 +195,10 @@ contract EVCUtilTest is Test {
         evcClient.calledByEVCWithChecksInProgress();
     }
 
-    function test_calledByEVCAccount_calledByEVCAccountOwner(address caller, uint8 id) external {
+    function test_calledByEVCAccount_calledByEVCAccountOwner_msgSenderOnlyEVCAccountOwner(
+        address caller,
+        uint8 id
+    ) external {
         vm.assume(!evc.haveCommonOwner(caller, address(0)) && !evc.haveCommonOwner(caller, address(evc)));
         vm.assume(id != 0);
 
@@ -203,6 +210,9 @@ contract EVCUtilTest is Test {
         vm.prank(caller);
         evcClient.calledByEVCAccountOwner();
 
+        vm.prank(caller);
+        assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller);
+
         // msg.sender is EVC and operator is authenticated
         evc.setOperatorAuthenticated(true);
         vm.prank(address(evc));
@@ -213,6 +223,10 @@ contract EVCUtilTest is Test {
         vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
         evcClient.calledByEVCAccountOwner();
 
+        vm.prank(address(evc));
+        vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
+        evcClient.msgSenderOnlyEVCAccountOwner();
+
         // msg.sender is EVC and control collateral is in progress
         evc.setOperatorAuthenticated(false);
         evc.setControlCollateralInProgress(true);
@@ -224,6 +238,10 @@ contract EVCUtilTest is Test {
         vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
         evcClient.calledByEVCAccountOwner();
 
+        vm.prank(address(evc));
+        vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
+        evcClient.msgSenderOnlyEVCAccountOwner();
+
         // msg.sender is EVC and checks are in progress
         evc.setControlCollateralInProgress(false);
         evc.setChecksInProgress(true);
@@ -235,6 +253,10 @@ contract EVCUtilTest is Test {
         vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
         evcClient.calledByEVCAccountOwner();
 
+        vm.prank(address(evc));
+        vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
+        evcClient.msgSenderOnlyEVCAccountOwner();
+
         // msg.sender is EVC, the owner is not registered yet
         evc.setChecksInProgress(false);
         evc.setOnBehalfOfAccount(caller);
@@ -246,6 +268,10 @@ contract EVCUtilTest is Test {
         vm.prank(address(evc));
         evcClient.calledByEVCAccountOwner();
 
+        assertEq(evc.getAccountOwner(caller), address(0));
+        vm.prank(address(evc));
+        assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller);
+
         // msg.sender is EVC, the owner is registered and the authenticated account is the owner
         evc.setOnBehalfOfAccount(address(0));
         vm.prank(caller);
@@ -256,10 +282,13 @@ contract EVCUtilTest is Test {
         evcClient.calledByEVCAccount();
 
         assertEq(evc.getAccountOwner(caller), caller);
-        evc.setOnBehalfOfAccount(caller);
         vm.prank(address(evc));
         evcClient.calledByEVCAccountOwner();
 
+        assertEq(evc.getAccountOwner(caller), caller);
+        vm.prank(address(evc));
+        assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller);
+
         // msg.sender is EVC, the owner is registered but the authenticated account is not the owner
         evc.setOnBehalfOfAccount(address(uint160(uint160(caller) ^ id)));
         vm.prank(address(evc));
@@ -268,6 +297,10 @@ contract EVCUtilTest is Test {
         vm.prank(address(evc));
         vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
         evcClient.calledByEVCAccountOwner();
+
+        vm.prank(address(evc));
+        vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector));
+        evcClient.msgSenderOnlyEVCAccountOwner();
     }
 
     function test_msgSender(address caller) external {