Update Adapter Access Control Logic

framework/contracts/account/src/contract.rs

@@ -25,18 +25,19 @@ use abstract_std::{
 };
 use cosmwasm_std::{
     ensure_eq, wasm_execute, Addr, Binary, Coins, Deps, DepsMut, Env, MessageInfo, Reply, Response,
-    StdResult, SubMsgResult,
+    StdResult,
 };
 
 pub use crate::migrate::migrate;
 use crate::{
     config::{update_account_status, update_info, update_internal_config},
     error::AccountError,
     execution::{
-        execute_ibc_action, execute_module_action, execute_module_action_response, ica_action,
+        admin_execute, admin_execute_on_module, execute_ibc_action, execute_msgs,
+        execute_msgs_with_data, execute_on_module, ica_action,
     },
     modules::{
-        _install_modules, exec_on_module, install_modules,
+        _install_modules, install_modules,
         migration::{handle_callback, upgrade_modules},
         uninstall_module, MIGRATE_CONTEXT,
     },
@@ -45,7 +46,7 @@ use crate::{
         handle_module_info_query, handle_module_versions_query, handle_sub_accounts_query,
         handle_top_level_owner_query,
     },
-    reply::{forward_response_data, register_dependencies},
+    reply::{admin_action_reply, forward_response_reply, register_dependencies},
     sub_account::{
         create_sub_account, handle_sub_account_action, maybe_update_sub_account_governance,
         remove_account_from_contracts,
@@ -59,8 +60,9 @@ pub type AccountResult<R = Response> = Result<R, AccountError>;
 
 pub const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION");
 
-pub const RESPONSE_REPLY_ID: u64 = 1;
-pub const REGISTER_MODULES_DEPENDENCIES_REPLY_ID: u64 = 2;
+pub const FORWARD_RESPONSE_REPLY_ID: u64 = 1;
+pub const ADMIN_ACTION_REPLY_ID: u64 = 2;
+pub const REGISTER_MODULES_DEPENDENCIES_REPLY_ID: u64 = 3;
 
 #[cfg_attr(feature = "export", cosmwasm_std::entry_point)]
 pub fn instantiate(
@@ -242,13 +244,39 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg)
             is_suspended: suspension_status,
         } => update_account_status(deps, info, suspension_status).map_err(AccountError::from),
         msg => {
-            // Block actions if user is not subscribed
+            // Block actions if account is suspended
             let is_suspended = SUSPENSION_STATUS.load(deps.storage)?;
             if is_suspended {
                 return Err(AccountError::AccountSuspended {});
             }
 
             match msg {
+                // ## Execution ##
+                ExecuteMsg::Execute { msgs } => {
+                    execute_msgs(deps, &info.sender, msgs).map_err(AccountError::from)
+                }
+                ExecuteMsg::AdminExecute { addr, msg } => {
+                    let addr = deps.api.addr_validate(&addr)?;
+                    admin_execute(deps, info, addr, msg)
+                }
+                ExecuteMsg::ExecuteWithData { msg } => {
+                    execute_msgs_with_data(deps, &info.sender, msg).map_err(AccountError::from)
+                }
+                ExecuteMsg::ExecuteOnModule {
+                    module_id,
+                    exec_msg,
+                } => execute_on_module(deps, info, module_id, exec_msg).map_err(AccountError::from),
+                ExecuteMsg::AdminExecuteOnModule { module_id, msg } => {
+                    admin_execute_on_module(deps, info, module_id, msg)
+                }
+                ExecuteMsg::IbcAction { msg } => {
+                    execute_ibc_action(deps, info, msg).map_err(AccountError::from)
+                }
+                ExecuteMsg::IcaAction { action_query_msg } => {
+                    ica_action(deps, info, action_query_msg).map_err(AccountError::from)
+                }
+
+                // ## Configuration ##
                 ExecuteMsg::UpdateInternalConfig(config) => {
                     update_internal_config(deps, info, config).map_err(AccountError::from)
                 }
@@ -258,29 +286,6 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg)
                 ExecuteMsg::UninstallModule { module_id } => {
                     uninstall_module(deps, info, module_id).map_err(AccountError::from)
                 }
-                ExecuteMsg::ExecOnModule {
-                    module_id,
-                    exec_msg,
-                } => exec_on_module(deps, info, module_id, exec_msg).map_err(AccountError::from),
-                ExecuteMsg::CreateSubAccount {
-                    name,
-                    description,
-                    link,
-                    namespace,
-                    install_modules,
-                    account_id,
-                } => create_sub_account(
-                    deps,
-                    info,
-                    env,
-                    name,
-                    description,
-                    link,
-                    namespace,
-                    install_modules,
-                    account_id,
-                )
-                .map_err(AccountError::from),
                 ExecuteMsg::Upgrade { modules } => {
                     upgrade_modules(deps, env, info, modules).map_err(AccountError::from)
                 }
@@ -289,12 +294,6 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg)
                     description,
                     link,
                 } => update_info(deps, info, name, description, link).map_err(AccountError::from),
-                ExecuteMsg::UpdateSubAccount(action) => {
-                    handle_sub_account_action(deps, info, action).map_err(AccountError::from)
-                }
-                // TODO: Update module migrate logic to not use callback!
-                // ExecuteMsg::Callback(CallbackMsg {}) => handle_callback(deps, env, info),
-                // Used to claim or renounce an ownership change.
                 ExecuteMsg::UpdateOwnership(action) => {
                     // If sub-account related it may require some messages to be constructed beforehand
                     let msgs = match &action {
@@ -321,40 +320,49 @@ pub fn execute(mut deps: DepsMut, env: Env, info: MessageInfo, msg: ExecuteMsg)
                             .add_messages(msgs),
                     )
                 }
-                ExecuteMsg::ModuleAction { msgs } => {
-                    execute_module_action(deps, info, msgs).map_err(AccountError::from)
-                }
-                ExecuteMsg::ModuleActionWithData { msg } => {
-                    execute_module_action_response(deps, info, msg).map_err(AccountError::from)
-                }
-                ExecuteMsg::IbcAction { msg } => {
-                    execute_ibc_action(deps, info, msg).map_err(AccountError::from)
-                }
-                ExecuteMsg::IcaAction { action_query_msg } => {
-                    ica_action(deps, info, action_query_msg).map_err(AccountError::from)
+
+                // ## Sub-Accounts ##
+                ExecuteMsg::CreateSubAccount {
+                    name,
+                    description,
+                    link,
+                    namespace,
+                    install_modules,
+                    account_id,
+                } => create_sub_account(
+                    deps,
+                    info,
+                    env,
+                    name,
+                    description,
+                    link,
+                    namespace,
+                    install_modules,
+                    account_id,
+                )
+                .map_err(AccountError::from),
+                ExecuteMsg::UpdateSubAccount(action) => {
+                    handle_sub_account_action(deps, info, action).map_err(AccountError::from)
                 }
+
+                // ## Other ##
+                // TODO: Update module migrate logic to not use callback!
+                ExecuteMsg::Callback(_) => handle_callback(deps, env, info),
                 ExecuteMsg::UpdateStatus { is_suspended: _ } => {
                     unreachable!("Update status case is reached above")
                 }
-                ExecuteMsg::Callback(_) => handle_callback(deps, env, info),
             }
         }
     }
 }
 
 #[cfg_attr(feature = "export", cosmwasm_std::entry_point)]
 pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> AccountResult {
-    match msg {
-        Reply {
-            id: RESPONSE_REPLY_ID,
-            result: SubMsgResult::Ok(_),
-            ..
-        } => forward_response_data(msg),
-        Reply {
-            id: REGISTER_MODULES_DEPENDENCIES_REPLY_ID,
-            result: SubMsgResult::Ok(_),
-            ..
-        } => register_dependencies(deps),
+    match msg.id {
+        FORWARD_RESPONSE_REPLY_ID => forward_response_reply(msg),
+        ADMIN_ACTION_REPLY_ID => admin_action_reply(deps),
+        REGISTER_MODULES_DEPENDENCIES_REPLY_ID => register_dependencies(deps),
+
         _ => Err(AccountError::UnexpectedReply {}),
     }
 }

framework/contracts/account/src/execution.rs

@@ -1,55 +1,100 @@
 use abstract_sdk::std::{
     account::state::WHITELISTED_MODULES, ibc_client::ExecuteMsg as IbcClientMsg, IBC_CLIENT,
 };
-use abstract_std::{account::state::ACCOUNT_MODULES, objects::ownership, ICA_CLIENT};
+use abstract_std::{
+    account::state::{ACCOUNT_MODULES, CALLING_TO_AS_ADMIN},
+    objects::ownership,
+    ICA_CLIENT,
+};
 use cosmwasm_std::{
     wasm_execute, Addr, Binary, CosmosMsg, Deps, DepsMut, Empty, MessageInfo, StdError, SubMsg,
-    WasmQuery,
+    WasmMsg, WasmQuery,
 };
 
 use crate::{
-    contract::{AccountResponse, AccountResult, RESPONSE_REPLY_ID},
+    contract::{AccountResponse, AccountResult, ADMIN_ACTION_REPLY_ID, FORWARD_RESPONSE_REPLY_ID},
     error::AccountError,
+    modules::load_module_addr,
 };
 
-/// Check that sender either whitelisted or governance
-pub(crate) fn assert_whitelisted_or_owner(deps: Deps, sender: &Addr) -> AccountResult<()> {
-    let whitelisted_modules = WHITELISTED_MODULES.load(deps.storage)?;
-    if whitelisted_modules.0.contains(sender)
-        || ownership::assert_nested_owner(deps.storage, &deps.querier, sender).is_ok()
-    {
-        Ok(())
-    } else {
-        Err(AccountError::SenderNotWhitelistedOrOwner {})
-    }
-}
-
 /// Executes `Vec<CosmosMsg>` on the proxy.
 /// Permission: Module
-pub fn execute_module_action(
+pub fn execute_msgs(
     deps: DepsMut,
-    msg_info: MessageInfo,
+    msg_sender: &Addr,
     msgs: Vec<CosmosMsg<Empty>>,
 ) -> AccountResult {
-    assert_whitelisted_or_owner(deps.as_ref(), &msg_info.sender)?;
+    assert_whitelisted_or_owner(deps.as_ref(), msg_sender)?;
 
     Ok(AccountResponse::action("execute_module_action").add_messages(msgs))
 }
 
 /// Executes `CosmosMsg` on the proxy and forwards its response.
 /// Permission: Module
-pub fn execute_module_action_response(
+pub fn execute_msgs_with_data(
     deps: DepsMut,
-    msg_info: MessageInfo,
+    msg_sender: &Addr,
     msg: CosmosMsg<Empty>,
 ) -> AccountResult {
-    assert_whitelisted_or_owner(deps.as_ref(), &msg_info.sender)?;
+    assert_whitelisted_or_owner(deps.as_ref(), msg_sender)?;
 
-    let submsg = SubMsg::reply_on_success(msg, RESPONSE_REPLY_ID);
+    let submsg = SubMsg::reply_on_success(msg, FORWARD_RESPONSE_REPLY_ID);
 
     Ok(AccountResponse::action("execute_module_action_response").add_submessage(submsg))
 }
 
+/// Execute the [`exec_msg`] on the provided [`module_id`],
+/// This is a simple wrapper around [`ExecuteMsg::Execute`](abstract_std::account::ExecuteMsg::Execute).
+pub fn execute_on_module(
+    deps: DepsMut,
+    info: MessageInfo,
+    module_id: String,
+    exec_msg: Binary,
+) -> AccountResult {
+    let module_addr = load_module_addr(deps.storage, &module_id)?;
+    execute_msgs(
+        deps,
+        &info.sender,
+        vec![CosmosMsg::Wasm(WasmMsg::Execute {
+            contract_addr: module_addr.into(),
+            msg: exec_msg,
+            funds: info.funds,
+        })],
+    )
+}
+
+pub fn admin_execute(
+    deps: DepsMut,
+    info: MessageInfo,
+    addr: Addr,
+    exec_msg: Binary,
+) -> AccountResult {
+    ownership::assert_nested_owner(deps.storage, &deps.querier, &info.sender)?;
+
+    CALLING_TO_AS_ADMIN.save(deps.storage, &addr)?;
+
+    let msg = SubMsg::reply_on_success(
+        WasmMsg::Execute {
+            contract_addr: addr.to_string(),
+            msg: exec_msg,
+            funds: info.funds,
+        },
+        ADMIN_ACTION_REPLY_ID,
+    );
+
+    Ok(AccountResponse::action("admin_execute").add_submessage(msg))
+}
+
+pub fn admin_execute_on_module(
+    deps: DepsMut,
+    info: MessageInfo,
+    module_id: String,
+    exec_msg: Binary,
+) -> AccountResult {
+    let module_addr = load_module_addr(deps.storage, &module_id)?;
+    admin_execute(deps, info, module_addr, exec_msg)
+}
+
 /// Executes IBC actions on the IBC client.
 /// Permission: Module
 pub fn execute_ibc_action(
@@ -105,6 +150,17 @@ pub fn ica_action(deps: DepsMut, msg_info: MessageInfo, action_query: Binary) ->
 
     Ok(AccountResponse::action("ica_action").add_messages(res.msgs))
 }
+/// Check that sender either whitelisted or governance
+pub(crate) fn assert_whitelisted_or_owner(deps: Deps, sender: &Addr) -> AccountResult<()> {
+    let whitelisted_modules = WHITELISTED_MODULES.load(deps.storage)?;
+    if whitelisted_modules.0.contains(sender)
+        || ownership::assert_nested_owner(deps.storage, &deps.querier, sender).is_ok()
+    {
+        Ok(())
+    } else {
+        Err(AccountError::SenderNotWhitelistedOrOwner {})
+    }
+}
 
 #[cfg(test)]
 mod test {
@@ -129,7 +185,7 @@ mod test {
             let mut deps = mock_dependencies();
             mock_init(&mut deps)?;
 
-            let msg = ExecuteMsg::ModuleAction { msgs: vec![] };
+            let msg = ExecuteMsg::Execute { msgs: vec![] };
 
             let info = message_info(&deps.api.addr_make("not_whitelisted"), &[]);
 
@@ -162,7 +218,7 @@ mod test {
             )?
             .into();
 
-            let msg = ExecuteMsg::ModuleAction {
+            let msg = ExecuteMsg::Execute {
                 msgs: vec![action.clone()],
             };