Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

account-sdk: include more context in the error type #389

Merged
merged 2 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions packages/account_sdk/src/account/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,30 @@ where
session,
}
}

pub async fn sign(
&self,
hash: FieldElement,
calls: &[Call],
) -> Result<RawSessionToken, SignError> {
let proofs = calls
.iter()
.map(|c| AllowedMethod {
contract_address: c.to,
selector: c.selector,
})
.map(|ref m| self.session.single_proof(m))
.collect::<Option<Vec<_>>>()
.ok_or(SignError::SessionMethodNotAllowed)?;
let mut proofs = Vec::new();

for call in calls {
let method = AllowedMethod {
selector: call.selector,
contract_address: call.to,
};

let Some(proof) = self.session.single_proof(&method) else {
return Err(SignError::SessionMethodNotAllowed {
selector: method.selector,
contract_address: method.contract_address,
});
};

proofs.push(proof);
}

Ok(RawSessionToken {
session: self.session.raw(),
session_authorization: self.session_authorization.clone(),
Expand All @@ -87,6 +97,7 @@ where
proofs,
})
}

fn session_magic() -> FieldElement {
short_string!("session-token")
}
Expand Down
18 changes: 16 additions & 2 deletions packages/account_sdk/src/signers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,28 @@ use self::webauthn::DeviceError;
pub enum SignError {
#[error("Signer error: {0}")]
Signer(EcdsaSignError),

#[error("Device error: {0}")]
Device(DeviceError),

#[error("NonAsciiName error: {0}")]
NonAsciiSessionNameError(#[from] NonAsciiNameError),

#[error("NoAllowedSessionMethods error")]
NoAllowedSessionMethods,
#[error("MethodNotAllowed error")]
SessionMethodNotAllowed,

/// Represents an error when trying to perform contract invocation that is not part
/// of a session's allowed methods.
#[error(
"Not allowed to call method selector `{selector:#x}` on contract `{contract_address:#x}`"
)]
SessionMethodNotAllowed {
/// The method selector that was not allowed.
selector: FieldElement,
/// The contract address the method was called on.
contract_address: FieldElement,
},

#[error("Invalid message provided: {0}")]
InvalidMessageError(String),
}
Expand Down
65 changes: 63 additions & 2 deletions packages/account_sdk/src/tests/account_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use crate::{
},
deploy_contract::FEE_TOKEN_ADDRESS,
signers::{
webauthn::internal::InternalWebauthnSigner, HashSigner, NewOwnerSigner, SignerTrait,
webauthn::internal::InternalWebauthnSigner, HashSigner, NewOwnerSigner, SignError,
SignerTrait,
},
tests::{
deployment_test::deploy_helper,
Expand All @@ -15,7 +16,7 @@ use crate::{
};
use cainome::cairo_serde::{ContractAddress, U256};
use starknet::{
accounts::Account,
accounts::{Account, AccountError},
macros::{felt, selector},
providers::Provider,
signers::SigningKey,
Expand Down Expand Up @@ -268,3 +269,63 @@ async fn test_change_owner_invalidate_old_sessions() {
.await
.unwrap();
}

#[tokio::test]
async fn test_call_unallowed_methods() {
let signer = InternalWebauthnSigner::random("localhost".to_string(), "rp_id".to_string());
let guardian = SigningKey::from_random();
let runner = KatanaRunner::load();
let address = deploy_helper(&runner, &signer, None as Option<&SigningKey>).await;
transfer_helper(&runner, &address).await;

let account = CartridgeGuardianAccount::new(
runner.client(),
signer.clone(),
guardian.clone(),
address,
runner.client().chain_id().await.unwrap(),
);

// Create random allowed method
let transfer_method = AllowedMethod::with_selector(*FEE_TOKEN_ADDRESS, selector!("transfer"));

let session_account = account
.session_account(
SigningKey::from_random(),
vec![transfer_method.clone()],
u64::MAX,
)
.await
.unwrap();

let contract = Erc20::new(*FEE_TOKEN_ADDRESS, &session_account);

let address = ContractAddress(address);
let amount = U256 {
high: 0,
low: 0x10_u128,
};

// calling allowed method should succeed
assert!(contract.transfer(&address, &amount).send().await.is_ok());

// Perform contract invocation that is not part of the allowed methods
let error = contract
.approve(&address, &amount)
.send()
.await
.unwrap_err();

// calling unallowed method should fail with `SessionMethodNotAllowed` error
let e @ AccountError::Signing(SignError::SessionMethodNotAllowed {
selector,
contract_address,
}) = error
else {
panic!("Expected `SessionMethodNotAllowed` error, got: {error:?}")
};

assert_eq!(selector!("approve"), selector);
assert_eq!(contract.address, contract_address);
assert!(e.to_string().contains("Not allowed to call method"));
}
Loading