Skip to content

Commit

Permalink
Change authentication contract errors to be auth errors. (#836)
Browse files Browse the repository at this point in the history
* Adapt authentication contract errors to be auth errors.

Returning contract errors introduces undesired confusion in terms of which entity has produced the error.
  • Loading branch information
dmkozh authored Jun 6, 2023
1 parent c09c5f1 commit b453eca
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 122 deletions.
15 changes: 15 additions & 0 deletions soroban-env-host/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -767,6 +767,21 @@ impl AuthorizationTracker {
if !self.authenticated {
let authenticate_res = self
.authenticate(host)
.map_err(|err| {
// Convert any contract errors to auth errors so that it's
// not possible to confuse them for the errors of the
// contract that has called `require_auth`.
if err.error.is_type(ScErrorType::Contract) {
host.err(
ScErrorType::Auth,
ScErrorCode::InvalidAction,
"failed account authentication",
&[err.error.to_raw()],
)
} else {
err
}
})
.and_then(|_| self.verify_nonce(host));
if let Some(err) = authenticate_res.err() {
self.is_valid = false;
Expand Down
217 changes: 95 additions & 122 deletions soroban-env-host/src/test/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1796,129 +1796,105 @@ fn test_classic_account_multisig_auth() {
.unwrap();

// Failure: only account weight (40)
assert_eq!(
to_contract_err(
token
.transfer(
&TestSigner::account_with_multisig(&account_id, vec![&test.user_key]),
receiver.clone(),
100,
)
.err()
.unwrap()
),
ContractError::AuthenticationError
);
assert!(token
.transfer(
&TestSigner::account_with_multisig(&account_id, vec![&test.user_key]),
receiver.clone(),
100,
)
.err()
.unwrap()
.error
.is_type(ScErrorType::Auth));

// Failure: 40 + 59 < 100
assert_eq!(
to_contract_err(
token
.transfer(
&TestSigner::account_with_multisig(
&account_id,
vec![&test.user_key, &test.user_key_4]
),
receiver.clone(),
100,
)
.err()
.unwrap()
),
ContractError::AuthenticationError
);
assert!(token
.transfer(
&TestSigner::account_with_multisig(&account_id, vec![&test.user_key, &test.user_key_4]),
receiver.clone(),
100,
)
.err()
.unwrap()
.error
.is_type(ScErrorType::Auth));

// Failure: 60 < 100, duplicate signatures
assert_eq!(
to_contract_err(
token
.transfer(
&TestSigner::account_with_multisig(
&account_id,
vec![&test.user_key_3, &test.user_key_3]
),
receiver.clone(),
100,
)
.err()
.unwrap()
),
ContractError::AuthenticationError
);
assert!(token
.transfer(
&TestSigner::account_with_multisig(
&account_id,
vec![&test.user_key_3, &test.user_key_3]
),
receiver.clone(),
100,
)
.err()
.unwrap()
.error
.is_type(ScErrorType::Auth));

// Failure: 60 + 59 > 100, duplicate signatures
assert_eq!(
to_contract_err(
token
.transfer(
&TestSigner::account_with_multisig(
&account_id,
vec![&test.user_key_3, &test.user_key_4, &test.user_key_3],
),
receiver.clone(),
100,
)
.err()
.unwrap()
),
ContractError::AuthenticationError
);
assert!(token
.transfer(
&TestSigner::account_with_multisig(
&account_id,
vec![&test.user_key_3, &test.user_key_4, &test.user_key_3],
),
receiver.clone(),
100,
)
.err()
.unwrap()
.error
.is_type(ScErrorType::Auth));

// Failure: 60 < 100 and incorrect signer
assert_eq!(
to_contract_err(
token
.transfer(
&TestSigner::account_with_multisig(
&account_id,
vec![&test.user_key_3, &test.issuer_key],
),
receiver.clone(),
100,
)
.err()
.unwrap()
),
ContractError::AuthenticationError
);
assert!(token
.transfer(
&TestSigner::account_with_multisig(
&account_id,
vec![&test.user_key_3, &test.issuer_key],
),
receiver.clone(),
100,
)
.err()
.unwrap()
.error
.is_type(ScErrorType::Auth));

// Failure: 60 + 59 > 100, but have incorrect signer
assert_eq!(
to_contract_err(
token
.transfer(
&TestSigner::account_with_multisig(
&account_id,
vec![&test.user_key_3, &test.user_key_4, &test.issuer_key],
),
receiver.clone(),
100,
)
.err()
.unwrap()
),
ContractError::AuthenticationError
);
assert!(token
.transfer(
&TestSigner::account_with_multisig(
&account_id,
vec![&test.user_key_3, &test.user_key_4, &test.issuer_key],
),
receiver.clone(),
100,
)
.err()
.unwrap()
.error
.is_type(ScErrorType::Auth));

// Failure: too many signatures (even though weight would be enough after
// deduplication).
let mut too_many_sigs = vec![];
for _ in 0..21 {
too_many_sigs.push(&test.user_key_2);
}
assert_eq!(
to_contract_err(
token
.transfer(
&TestSigner::account_with_multisig(&account_id, too_many_sigs,),
receiver.clone(),
100,
)
.err()
.unwrap()
),
ContractError::AuthenticationError
);
assert!(token
.transfer(
&TestSigner::account_with_multisig(&account_id, too_many_sigs,),
receiver.clone(),
100,
)
.err()
.unwrap()
.error
.is_type(ScErrorType::Auth));

// Failure: out of order signers
let mut out_of_order_signers = vec![
Expand All @@ -1929,22 +1905,19 @@ fn test_classic_account_multisig_auth() {
];
out_of_order_signers.sort_by_key(|k| k.public.as_bytes());
out_of_order_signers.swap(1, 2);
assert_eq!(
to_contract_err(
token
.transfer(
&TestSigner::Account(AccountSigner {
account_id: account_id,
signers: out_of_order_signers,
}),
receiver,
100,
)
.err()
.unwrap()
),
ContractError::AuthenticationError
);
assert!(token
.transfer(
&TestSigner::Account(AccountSigner {
account_id: account_id,
signers: out_of_order_signers,
}),
receiver,
100,
)
.err()
.unwrap()
.error
.is_type(ScErrorType::Auth));
}

#[test]
Expand Down

0 comments on commit b453eca

Please sign in to comment.