Skip to content

Commit

Permalink
Provide a way to allow non-root auth in recording mode. (#1059)
Browse files Browse the repository at this point in the history
### What

Provide a way to allow non-root auth in recording mode.

This is intentionally done via a separate helper with lengthy name and
not a `mock_all_auths` argument - this is intended to be an option for a
few 'exotic' contracts that do non-atomic bundling and majority of the
users shouldn't ever need that.

Resolves #1058

### Why

Recording auth now doesn't allow non-root `require_auth` by default in
order to prevent the potential user footguns. This PR brings back the
old behavior where non-root auth is allowed.

### Known limitations

N/A

Co-authored-by: Siddharth Suresh <siddharth@stellar.org>
  • Loading branch information
dmkozh and sisuresh authored Aug 22, 2023
1 parent ff71e74 commit d8bd392
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 4 deletions.
38 changes: 36 additions & 2 deletions soroban-sdk-macros/src/derive_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ pub fn derive_client_type(crate_path: &Path, ty: &str, name: &str) -> TokenStrea
#[doc(hidden)]
#[cfg(any(test, feature = "testutils"))]
mock_all_auths: bool,
#[doc(hidden)]
#[cfg(any(test, feature = "testutils"))]
allow_non_root_auth: bool,
}

impl<'a> #client_ident<'a> {
Expand All @@ -41,6 +44,8 @@ pub fn derive_client_type(crate_path: &Path, ty: &str, name: &str) -> TokenStrea
mock_auths: None,
#[cfg(any(test, feature = "testutils"))]
mock_all_auths: false,
#[cfg(any(test, feature = "testutils"))]
allow_non_root_auth: false,
}
}

Expand All @@ -60,6 +65,7 @@ pub fn derive_client_type(crate_path: &Path, ty: &str, name: &str) -> TokenStrea
set_auths: Some(auths),
mock_auths: self.mock_auths.clone(),
mock_all_auths: false,
allow_non_root_auth: false,
}
}

Expand All @@ -76,14 +82,16 @@ pub fn derive_client_type(crate_path: &Path, ty: &str, name: &str) -> TokenStrea
set_auths: self.set_auths.clone(),
mock_auths: Some(mock_auths),
mock_all_auths: false,
allow_non_root_auth: false,
}
}

/// Mock all calls to the `Address::require_auth` and
/// `Address::require_auth_for_args` functions in invoked contracts,
/// having them succeed as if authorization was provided.
///
/// See `soroban_sdk::Env::set_auths` for more details and examples.
/// See `soroban_sdk::Env::mock_all_auths` for more details and
/// examples.
#[cfg(any(test, feature = "testutils"))]
pub fn mock_all_auths(&self) -> Self {
Self {
Expand All @@ -92,6 +100,28 @@ pub fn derive_client_type(crate_path: &Path, ty: &str, name: &str) -> TokenStrea
set_auths: None,
mock_auths: None,
mock_all_auths: true,
allow_non_root_auth: false,
}
}

/// A version of `mock_all_auths` that allows authorizations that
/// are not present in the root invocation.
///
/// Refer to `mock_all_auths` documentation for details and
/// prefer using `mock_all_auths` unless non-root authorization is
/// required.
///
/// See `soroban_sdk::Env::mock_all_auths_allowing_non_root_auth`
/// for more details and examples.
#[cfg(any(test, feature = "testutils"))]
pub fn mock_all_auths_allowing_non_root_auth(&self) -> Self {
Self {
env: self.env.clone(),
address: self.address.clone(),
set_auths: None,
mock_auths: None,
mock_all_auths: true,
allow_non_root_auth: true,
}
}
}
Expand Down Expand Up @@ -163,7 +193,11 @@ pub fn derive_client_impl(crate_path: &Path, name: &str, fns: &[syn_ext::Fn]) ->
self.env.mock_auths(mock_auths);
}
if self.mock_all_auths {
self.env.mock_all_auths();
if self.allow_non_root_auth {
self.env.mock_all_auths_allowing_non_root_auth();
} else {
self.env.mock_all_auths();
}
}
}
use #crate_path::{IntoVal,FromVal};
Expand Down
55 changes: 55 additions & 0 deletions soroban-sdk/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,61 @@ impl Env {
self.env_impl.switch_to_recording_auth(true).unwrap();
}

/// A version of `mock_all_auths` that allows authorizations that are not
/// present in the root invocation.
///
/// Refer to `mock_all_auths` documentation for general information and
/// prefer using `mock_all_auths` unless non-root authorization is required.
///
/// The only difference from `mock_all_auths` is that this won't return an
/// error when `require_auth` hasn't been called in the root invocation for
/// any given address. This is useful to test contracts that bundle calls to
/// another contract without atomicity requirements (i.e. any contract call
/// can be frontrun).
///
/// ### Examples
/// ```
/// use soroban_sdk::{contract, contractimpl, Env, Address, testutils::Address as _};
///
/// #[contract]
/// pub struct ContractA;
///
/// #[contractimpl]
/// impl ContractA {
/// pub fn do_auth(env: Env, addr: Address) {
/// addr.require_auth();
/// }
/// }
/// #[contract]
/// pub struct ContractB;
///
/// #[contractimpl]
/// impl ContractB {
/// pub fn call_a(env: Env, contract_a: Address, addr: Address) {
/// // Notice there is no `require_auth` call here.
/// ContractAClient::new(&env, &contract_a).do_auth(&addr);
/// }
/// }
/// #[test]
/// fn test() {
/// # }
/// # fn main() {
/// let env = Env::default();
/// let contract_a = env.register_contract(None, ContractA);
/// let contract_b = env.register_contract(None, ContractB);
/// // The regular `env.mock_all_auths()` would result in the call
/// // failure.
/// env.mock_all_auths_allowing_non_root_auth();
///
/// let client = ContractBClient::new(&env, &contract_b);
/// let addr = Address::random(&env);
/// client.call_a(&contract_a, &addr);
/// }
/// ```
pub fn mock_all_auths_allowing_non_root_auth(&self) {
self.env_impl.switch_to_recording_auth(false).unwrap();
}

/// Returns a list of authorization trees that were seen during the last
/// contract or authorized host function invocation.
///
Expand Down
12 changes: 10 additions & 2 deletions soroban-sdk/src/tests/token_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ impl TestContract {
}

#[test]
#[should_panic] // FIXME: this is not supposed to panic; it does due to auth failure, dmkoz to fix.
fn test_mock_all_auth() {
extern crate std;

Expand All @@ -60,7 +59,16 @@ fn test_mock_all_auth() {
let from = Address::random(&env);
let spender = Address::random(&env);

client.mock_all_auths().approve(&from, &spender, &20, &200);
// `TestContract` doesn't call `require_auth` before calling into token,
// thus we need to allow non-root auth and regular `mock_all_auths` will
// fail.
assert!(client
.mock_all_auths()
.try_approve(&from, &spender, &20, &200)
.is_err());
client
.mock_all_auths_allowing_non_root_auth()
.approve(&from, &spender, &20, &200);

assert_eq!(
env.auths(),
Expand Down

0 comments on commit d8bd392

Please sign in to comment.