Skip to content
Open
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
25 changes: 25 additions & 0 deletions cmd/crates/soroban-test/tests/it/integration/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,31 @@ async fn standard_auth_with_separate_signer() {
.stdout("\"hello\"\n");
}

// Regression test for https://github.com/stellar/stellar-cli/issues/2459:
// passing a signer by its public key (G...) must resolve to the stored
// identity that holds it, exactly like passing the identity alias does.
#[tokio::test]
async fn standard_auth_with_separate_signer_by_public_key() {
let sandbox = &TestEnv::new();
let signer_pubkey = new_account(sandbox, "signer");

let (id, _) = deploy_auth_contracts(sandbox).await;

sandbox
.new_assert_cmd("contract")
.arg("invoke")
.arg("--source=test")
.arg("--id")
.arg(&id)
.arg("--")
.arg("do-auth")
.arg(format!("--addr={signer_pubkey}"))
.arg("--val=hello")
.assert()
.success()
.stdout("\"hello\"\n");
}

#[tokio::test]
async fn root_auth_with_authorized_subcall() {
let sandbox = &TestEnv::new();
Expand Down
31 changes: 28 additions & 3 deletions cmd/crates/soroban-test/tests/it/integration/hello_world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ async fn invoke_contract() {
invoke_auth_with_identity(sandbox, id, "test", &addr);
invoke_auth_with_identity(sandbox, id, "testone", &addr_1);
invoke_auth_with_non_source_identity(sandbox, id, "test", "testone", &addr_1);
invoke_auth_with_different_test_account_fail(sandbox, id, &addr_1).await;
invoke_auth_with_unknown_account_fail(sandbox, id).await;
contract_data_read_failure(sandbox, id);
invoke_with_seed(sandbox, id, &seed_phrase).await;
invoke_with_sk(sandbox, id, &secret_key).await;
Expand Down Expand Up @@ -249,10 +249,35 @@ fn invoke_auth_with_non_source_identity(
.success();
}

async fn invoke_auth_with_different_test_account_fail(sandbox: &TestEnv, id: &str, addr: &str) {
// A public key that matches no stored identity has no signer, so requiring its
// auth must fail rather than silently succeed. This guards the `CannotSign`
// fallback in `resolve_secret`: signing by public key only works when the key
// belongs to a known identity.
async fn invoke_auth_with_unknown_account_fail(sandbox: &TestEnv, id: &str) {
// Mint a fresh keypair, capture its address, then remove the identity so the
// address is a valid, funded-elsewhere account that we hold no secret for.
sandbox
.new_assert_cmd("keys")
.arg("generate")
.arg("unknown")
.assert()
.success();
let addr = sandbox
.new_assert_cmd("keys")
.arg("address")
.arg("unknown")
.assert()
.stdout_as_str();
sandbox
.new_assert_cmd("keys")
.arg("rm")
.arg("--force")
.arg("unknown")
.assert()
.success();

let res = sandbox
.invoke_with_test(&[
"--hd-path=0",
"--id",
id,
"--",
Expand Down
10 changes: 7 additions & 3 deletions cmd/soroban-cli/src/commands/contract/arg_parsing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,9 +465,13 @@ fn resolve_address(addr_or_alias: &str, config: &config::Args) -> Result<String,
}

fn resolve_signer(addr_or_alias: &str, config: &config::Args) -> Option<Signer> {
let secret = config.locator.get_secret_key(addr_or_alias).ok()?;
let print = Print::new(false);
let signer = secret.signer(config.hd_path(), print).ok()?;
let account: config::UnresolvedMuxedAccount = addr_or_alias.parse().ok()?;
// A raw public key is matched to an identity at `--hd-path`, so the signer
// is built at that same path; an alias or secret honors it the same way.
let secret = account
.resolve_secret(&config.locator, config.hd_path())
.ok()?;
let signer = secret.signer(config.hd_path(), Print::new(false)).ok()?;
Some(signer)
}

Expand Down
82 changes: 80 additions & 2 deletions cmd/soroban-cli/src/config/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,26 @@ impl UnresolvedMuxedAccount {
}
}

pub fn resolve_secret(&self, locator: &locator::Args) -> Result<secret::Secret, Error> {
pub fn resolve_secret(
&self,
locator: &locator::Args,
hd_path: Option<u32>,
) -> Result<secret::Secret, Error> {
match &self {
// A literal public key has no secret on its own, but a stored
// identity may hold the matching key. Scan identities by public key
// so `G...` signs like its alias would; fall back to `CannotSign`
// when nothing matches. Muxed accounts (`M...`) aren't signable
// end-to-end yet (see the `todo!` in `sign_soroban_authorizations`),
// so they keep returning `CannotSign`.
UnresolvedMuxedAccount::Resolved(muxed_account) => {
Err(Error::CannotSign(muxed_account.clone()))
let xdr::MuxedAccount::Ed25519(xdr::Uint256(key)) = muxed_account else {
return Err(Error::CannotSign(muxed_account.clone()));
};
let target = stellar_strkey::ed25519::PublicKey(*key);
locator
.secret_by_public_key(&target, hd_path)?
.ok_or_else(|| Error::CannotSign(muxed_account.clone()))
}
UnresolvedMuxedAccount::AliasOrSecret(alias_or_secret) => {
Ok(locator.read_key(alias_or_secret)?.try_into()?)
Expand Down Expand Up @@ -200,6 +216,68 @@ impl AsRef<std::path::Path> for ContractName {
#[cfg(test)]
mod tests {
use super::*;
use crate::config::secret::Secret;

const TEST_PUBLIC_KEY: &str = "GAREAZZQWHOCBJS236KIE3AWYBVFLSBK7E5UW3ICI3TCRWQKT5LNLCEZ";
const TEST_SECRET_KEY: &str = "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BH";
const OTHER_PUBLIC_KEY: &str = "GAKSH6AD2IPJQELTHIOWDAPYX74YELUOWJLI2L4RIPIPZH6YQIFNUSDC";

fn locator_with_identity() -> (tempfile::TempDir, locator::Args) {
let dir = tempfile::tempdir().unwrap();
let locator = locator::Args {
config_dir: Some(dir.path().to_path_buf()),
};
let secret = Secret::SecretKey {
secret_key: TEST_SECRET_KEY.to_string(),
};
locator.write_identity("alice", &secret).unwrap();
(dir, locator)
}

#[test]
fn resolve_secret_matches_public_key_to_stored_identity() {
let (_dir, locator) = locator_with_identity();
let account: UnresolvedMuxedAccount = TEST_PUBLIC_KEY.parse().unwrap();
assert!(matches!(account, UnresolvedMuxedAccount::Resolved(_)));

let secret = account.resolve_secret(&locator, None).unwrap();
assert!(matches!(
secret,
Secret::SecretKey { ref secret_key } if secret_key == TEST_SECRET_KEY
));
}

#[test]
fn resolve_secret_errors_when_public_key_has_no_stored_identity() {
let (_dir, locator) = locator_with_identity();
let account: UnresolvedMuxedAccount = OTHER_PUBLIC_KEY.parse().unwrap();

assert!(matches!(
account.resolve_secret(&locator, None).unwrap_err(),
Error::CannotSign(_)
));
}

#[test]
fn resolve_secret_rejects_muxed_account_even_with_stored_identity() {
let (_dir, locator) = locator_with_identity();
// A muxed account (`M...`) wrapping alice's ed25519 key. Even though the
// underlying key belongs to a stored identity, muxed accounts aren't
// signable end-to-end yet, so resolution must still return `CannotSign`
// rather than the stored secret.
let pk = stellar_strkey::ed25519::PublicKey::from_string(TEST_PUBLIC_KEY).unwrap();
let account = UnresolvedMuxedAccount::Resolved(xdr::MuxedAccount::MuxedEd25519(
xdr::MuxedAccountMed25519 {
id: 1,
ed25519: xdr::Uint256(pk.0),
},
));

assert!(matches!(
account.resolve_secret(&locator, None).unwrap_err(),
Error::CannotSign(_)
));
}

#[test]
fn ledger_shorthand_is_not_recognized() {
Expand Down
94 changes: 94 additions & 0 deletions cmd/soroban-cli/src/config/locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,28 @@ impl Args {
Ok(self.read_key(key_or_name)?.muxed_account(hd_path)?)
}

/// Find a stored identity whose public key matches `target`, returning its
/// secret. Each identity is derived at `hd_path` (falling back to its own
/// persisted path when `hd_path` is `None`), so a key looked up by strkey
/// resolves the same way it would by alias under the same `--hd-path`.
/// Best-effort: identities whose public key can't be derived without error
/// (e.g. a disconnected ledger) are skipped rather than failing the lookup.
pub fn secret_by_public_key(
&self,
target: &stellar_strkey::ed25519::PublicKey,
hd_path: Option<u32>,
) -> Result<Option<Secret>, Error> {
for name in self.list_identities()? {
let Ok(Key::Secret(secret)) = self.read_identity(&name) else {
continue;
};
if secret.public_key(hd_path).is_ok_and(|pk| &pk == target) {
return Ok(Some(secret));
}
}
Ok(None)
}

pub fn read_network(&self, name: &str) -> Result<Network, Error> {
utils::validate_name(name)?;
let res = KeyType::Network.read_with_global(name, self);
Expand Down Expand Up @@ -1525,4 +1547,76 @@ mod tests {
assert!(matches!(key, Key::PublicKey(_)));
}
}

mod secret_by_public_key {
use super::super::*;

const TEST_PUBLIC_KEY: &str = "GAREAZZQWHOCBJS236KIE3AWYBVFLSBK7E5UW3ICI3TCRWQKT5LNLCEZ";
const TEST_SECRET_KEY: &str = "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BH";
const OTHER_PUBLIC_KEY: &str = "GAKSH6AD2IPJQELTHIOWDAPYX74YELUOWJLI2L4RIPIPZH6YQIFNUSDC";
const TEST_SEED_PHRASE: &str =
"depth decade power loud smile spatial sign movie judge february rate broccoli";

fn locator_with_tempdir() -> (tempfile::TempDir, Args) {
let dir = tempfile::tempdir().unwrap();
let args = Args {
config_dir: Some(dir.path().to_path_buf()),
};
(dir, args)
}

#[test]
fn returns_secret_for_stored_identity() {
let (_dir, locator) = locator_with_tempdir();
let secret = Secret::SecretKey {
secret_key: TEST_SECRET_KEY.to_string(),
};
locator.write_identity("alice", &secret).unwrap();

let target = stellar_strkey::ed25519::PublicKey::from_string(TEST_PUBLIC_KEY).unwrap();
let found = locator.secret_by_public_key(&target, None).unwrap();

assert!(matches!(
found,
Some(Secret::SecretKey { ref secret_key }) if secret_key == TEST_SECRET_KEY
));
}

#[test]
fn returns_none_for_unknown_public_key() {
let (_dir, locator) = locator_with_tempdir();
let secret = Secret::SecretKey {
secret_key: TEST_SECRET_KEY.to_string(),
};
locator.write_identity("alice", &secret).unwrap();

let target = stellar_strkey::ed25519::PublicKey::from_string(OTHER_PUBLIC_KEY).unwrap();
assert!(locator
.secret_by_public_key(&target, None)
.unwrap()
.is_none());
}

#[test]
fn matches_identity_at_requested_hd_path() {
let (_dir, locator) = locator_with_tempdir();
let secret = Secret::SeedPhrase {
seed_phrase: TEST_SEED_PHRASE.to_string(),
hd_path: None,
};
locator.write_identity("alice", &secret).unwrap();

// The account derived at index 5 is only found when the lookup uses
// the same hd_path; the default (index 0) path must not match it.
let at_five = secret.public_key(Some(5)).unwrap();
assert!(locator
.secret_by_public_key(&at_five, Some(5))
.unwrap()
.is_some());
assert!(locator
.secret_by_public_key(&at_five, None)
.unwrap()
.is_none());
}
}
}
4 changes: 3 additions & 1 deletion cmd/soroban-cli/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ impl Args {
}

pub fn key_pair(&self) -> Result<ed25519_dalek::SigningKey, Error> {
let key = &self.source_account.resolve_secret(&self.locator)?;
let key = &self
.source_account
.resolve_secret(&self.locator, self.hd_path())?;
Ok(key.key_pair(self.hd_path())?)
}

Expand Down
Loading