Skip to content

Commit 6c3bf44

Browse files
fnandofnando
authored
Sign auth entries when the signer is passed by public key (#2658)
1 parent cbe70c3 commit 6c3bf44

6 files changed

Lines changed: 237 additions & 9 deletions

File tree

‎cmd/crates/soroban-test/tests/it/integration/auth.rs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,31 @@ async fn standard_auth_with_separate_signer() {
5353
.stdout("\"hello\"\n");
5454
}
5555

56+
// Regression test for https://github.com/stellar/stellar-cli/issues/2459:
57+
// passing a signer by its public key (G...) must resolve to the stored
58+
// identity that holds it, exactly like passing the identity alias does.
59+
#[tokio::test]
60+
async fn standard_auth_with_separate_signer_by_public_key() {
61+
let sandbox = &TestEnv::new();
62+
let signer_pubkey = new_account(sandbox, "signer");
63+
64+
let (id, _) = deploy_auth_contracts(sandbox).await;
65+
66+
sandbox
67+
.new_assert_cmd("contract")
68+
.arg("invoke")
69+
.arg("--source=test")
70+
.arg("--id")
71+
.arg(&id)
72+
.arg("--")
73+
.arg("do-auth")
74+
.arg(format!("--addr={signer_pubkey}"))
75+
.arg("--val=hello")
76+
.assert()
77+
.success()
78+
.stdout("\"hello\"\n");
79+
}
80+
5681
#[tokio::test]
5782
async fn root_auth_with_authorized_subcall() {
5883
let sandbox = &TestEnv::new();

‎cmd/crates/soroban-test/tests/it/integration/hello_world.rs‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ async fn invoke_contract() {
134134
invoke_auth_with_identity(sandbox, id, "test", &addr);
135135
invoke_auth_with_identity(sandbox, id, "testone", &addr_1);
136136
invoke_auth_with_non_source_identity(sandbox, id, "test", "testone", &addr_1);
137-
invoke_auth_with_different_test_account_fail(sandbox, id, &addr_1).await;
137+
invoke_auth_with_unknown_account_fail(sandbox, id).await;
138138
contract_data_read_failure(sandbox, id);
139139
invoke_with_seed(sandbox, id, &seed_phrase).await;
140140
invoke_with_sk(sandbox, id, &secret_key).await;
@@ -249,10 +249,35 @@ fn invoke_auth_with_non_source_identity(
249249
.success();
250250
}
251251

252-
async fn invoke_auth_with_different_test_account_fail(sandbox: &TestEnv, id: &str, addr: &str) {
252+
// A public key that matches no stored identity has no signer, so requiring its
253+
// auth must fail rather than silently succeed. This guards the `CannotSign`
254+
// fallback in `resolve_secret`: signing by public key only works when the key
255+
// belongs to a known identity.
256+
async fn invoke_auth_with_unknown_account_fail(sandbox: &TestEnv, id: &str) {
257+
// Mint a fresh keypair, capture its address, then remove the identity so the
258+
// address is a valid, funded-elsewhere account that we hold no secret for.
259+
sandbox
260+
.new_assert_cmd("keys")
261+
.arg("generate")
262+
.arg("unknown")
263+
.assert()
264+
.success();
265+
let addr = sandbox
266+
.new_assert_cmd("keys")
267+
.arg("address")
268+
.arg("unknown")
269+
.assert()
270+
.stdout_as_str();
271+
sandbox
272+
.new_assert_cmd("keys")
273+
.arg("rm")
274+
.arg("--force")
275+
.arg("unknown")
276+
.assert()
277+
.success();
278+
253279
let res = sandbox
254280
.invoke_with_test(&[
255-
"--hd-path=0",
256281
"--id",
257282
id,
258283
"--",

‎cmd/soroban-cli/src/commands/contract/arg_parsing.rs‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -465,9 +465,13 @@ fn resolve_address(addr_or_alias: &str, config: &config::Args) -> Result<String,
465465
}
466466

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

‎cmd/soroban-cli/src/config/address.rs‎

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,26 @@ impl UnresolvedMuxedAccount {
7676
}
7777
}
7878

79-
pub fn resolve_secret(&self, locator: &locator::Args) -> Result<secret::Secret, Error> {
79+
pub fn resolve_secret(
80+
&self,
81+
locator: &locator::Args,
82+
hd_path: Option<u32>,
83+
) -> Result<secret::Secret, Error> {
8084
match &self {
85+
// A literal public key has no secret on its own, but a stored
86+
// identity may hold the matching key. Scan identities by public key
87+
// so `G...` signs like its alias would; fall back to `CannotSign`
88+
// when nothing matches. Muxed accounts (`M...`) aren't signable
89+
// end-to-end yet (see the `todo!` in `sign_soroban_authorizations`),
90+
// so they keep returning `CannotSign`.
8191
UnresolvedMuxedAccount::Resolved(muxed_account) => {
82-
Err(Error::CannotSign(muxed_account.clone()))
92+
let xdr::MuxedAccount::Ed25519(xdr::Uint256(key)) = muxed_account else {
93+
return Err(Error::CannotSign(muxed_account.clone()));
94+
};
95+
let target = stellar_strkey::ed25519::PublicKey(*key);
96+
locator
97+
.secret_by_public_key(&target, hd_path)?
98+
.ok_or_else(|| Error::CannotSign(muxed_account.clone()))
8399
}
84100
UnresolvedMuxedAccount::AliasOrSecret(alias_or_secret) => {
85101
Ok(locator.read_key(alias_or_secret)?.try_into()?)
@@ -200,6 +216,68 @@ impl AsRef<std::path::Path> for ContractName {
200216
#[cfg(test)]
201217
mod tests {
202218
use super::*;
219+
use crate::config::secret::Secret;
220+
221+
const TEST_PUBLIC_KEY: &str = "GAREAZZQWHOCBJS236KIE3AWYBVFLSBK7E5UW3ICI3TCRWQKT5LNLCEZ";
222+
const TEST_SECRET_KEY: &str = "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BH";
223+
const OTHER_PUBLIC_KEY: &str = "GAKSH6AD2IPJQELTHIOWDAPYX74YELUOWJLI2L4RIPIPZH6YQIFNUSDC";
224+
225+
fn locator_with_identity() -> (tempfile::TempDir, locator::Args) {
226+
let dir = tempfile::tempdir().unwrap();
227+
let locator = locator::Args {
228+
config_dir: Some(dir.path().to_path_buf()),
229+
};
230+
let secret = Secret::SecretKey {
231+
secret_key: TEST_SECRET_KEY.to_string(),
232+
};
233+
locator.write_identity("alice", &secret).unwrap();
234+
(dir, locator)
235+
}
236+
237+
#[test]
238+
fn resolve_secret_matches_public_key_to_stored_identity() {
239+
let (_dir, locator) = locator_with_identity();
240+
let account: UnresolvedMuxedAccount = TEST_PUBLIC_KEY.parse().unwrap();
241+
assert!(matches!(account, UnresolvedMuxedAccount::Resolved(_)));
242+
243+
let secret = account.resolve_secret(&locator, None).unwrap();
244+
assert!(matches!(
245+
secret,
246+
Secret::SecretKey { ref secret_key } if secret_key == TEST_SECRET_KEY
247+
));
248+
}
249+
250+
#[test]
251+
fn resolve_secret_errors_when_public_key_has_no_stored_identity() {
252+
let (_dir, locator) = locator_with_identity();
253+
let account: UnresolvedMuxedAccount = OTHER_PUBLIC_KEY.parse().unwrap();
254+
255+
assert!(matches!(
256+
account.resolve_secret(&locator, None).unwrap_err(),
257+
Error::CannotSign(_)
258+
));
259+
}
260+
261+
#[test]
262+
fn resolve_secret_rejects_muxed_account_even_with_stored_identity() {
263+
let (_dir, locator) = locator_with_identity();
264+
// A muxed account (`M...`) wrapping alice's ed25519 key. Even though the
265+
// underlying key belongs to a stored identity, muxed accounts aren't
266+
// signable end-to-end yet, so resolution must still return `CannotSign`
267+
// rather than the stored secret.
268+
let pk = stellar_strkey::ed25519::PublicKey::from_string(TEST_PUBLIC_KEY).unwrap();
269+
let account = UnresolvedMuxedAccount::Resolved(xdr::MuxedAccount::MuxedEd25519(
270+
xdr::MuxedAccountMed25519 {
271+
id: 1,
272+
ed25519: xdr::Uint256(pk.0),
273+
},
274+
));
275+
276+
assert!(matches!(
277+
account.resolve_secret(&locator, None).unwrap_err(),
278+
Error::CannotSign(_)
279+
));
280+
}
203281

204282
#[test]
205283
fn ledger_shorthand_is_not_recognized() {

‎cmd/soroban-cli/src/config/locator.rs‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,28 @@ impl Args {
401401
Ok(self.read_key(key_or_name)?.muxed_account(hd_path)?)
402402
}
403403

404+
/// Find a stored identity whose public key matches `target`, returning its
405+
/// secret. Each identity is derived at `hd_path` (falling back to its own
406+
/// persisted path when `hd_path` is `None`), so a key looked up by strkey
407+
/// resolves the same way it would by alias under the same `--hd-path`.
408+
/// Best-effort: identities whose public key can't be derived without error
409+
/// (e.g. a disconnected ledger) are skipped rather than failing the lookup.
410+
pub fn secret_by_public_key(
411+
&self,
412+
target: &stellar_strkey::ed25519::PublicKey,
413+
hd_path: Option<u32>,
414+
) -> Result<Option<Secret>, Error> {
415+
for name in self.list_identities()? {
416+
let Ok(Key::Secret(secret)) = self.read_identity(&name) else {
417+
continue;
418+
};
419+
if secret.public_key(hd_path).is_ok_and(|pk| &pk == target) {
420+
return Ok(Some(secret));
421+
}
422+
}
423+
Ok(None)
424+
}
425+
404426
pub fn read_network(&self, name: &str) -> Result<Network, Error> {
405427
utils::validate_name(name)?;
406428
let res = KeyType::Network.read_with_global(name, self);
@@ -1525,4 +1547,76 @@ mod tests {
15251547
assert!(matches!(key, Key::PublicKey(_)));
15261548
}
15271549
}
1550+
1551+
mod secret_by_public_key {
1552+
use super::super::*;
1553+
1554+
const TEST_PUBLIC_KEY: &str = "GAREAZZQWHOCBJS236KIE3AWYBVFLSBK7E5UW3ICI3TCRWQKT5LNLCEZ";
1555+
const TEST_SECRET_KEY: &str = "SBF5HLRREHMS36XZNTUSKZ6FTXDZGNXOHF4EXKUL5UCWZLPBX3NGJ4BH";
1556+
const OTHER_PUBLIC_KEY: &str = "GAKSH6AD2IPJQELTHIOWDAPYX74YELUOWJLI2L4RIPIPZH6YQIFNUSDC";
1557+
const TEST_SEED_PHRASE: &str =
1558+
"depth decade power loud smile spatial sign movie judge february rate broccoli";
1559+
1560+
fn locator_with_tempdir() -> (tempfile::TempDir, Args) {
1561+
let dir = tempfile::tempdir().unwrap();
1562+
let args = Args {
1563+
config_dir: Some(dir.path().to_path_buf()),
1564+
};
1565+
(dir, args)
1566+
}
1567+
1568+
#[test]
1569+
fn returns_secret_for_stored_identity() {
1570+
let (_dir, locator) = locator_with_tempdir();
1571+
let secret = Secret::SecretKey {
1572+
secret_key: TEST_SECRET_KEY.to_string(),
1573+
};
1574+
locator.write_identity("alice", &secret).unwrap();
1575+
1576+
let target = stellar_strkey::ed25519::PublicKey::from_string(TEST_PUBLIC_KEY).unwrap();
1577+
let found = locator.secret_by_public_key(&target, None).unwrap();
1578+
1579+
assert!(matches!(
1580+
found,
1581+
Some(Secret::SecretKey { ref secret_key }) if secret_key == TEST_SECRET_KEY
1582+
));
1583+
}
1584+
1585+
#[test]
1586+
fn returns_none_for_unknown_public_key() {
1587+
let (_dir, locator) = locator_with_tempdir();
1588+
let secret = Secret::SecretKey {
1589+
secret_key: TEST_SECRET_KEY.to_string(),
1590+
};
1591+
locator.write_identity("alice", &secret).unwrap();
1592+
1593+
let target = stellar_strkey::ed25519::PublicKey::from_string(OTHER_PUBLIC_KEY).unwrap();
1594+
assert!(locator
1595+
.secret_by_public_key(&target, None)
1596+
.unwrap()
1597+
.is_none());
1598+
}
1599+
1600+
#[test]
1601+
fn matches_identity_at_requested_hd_path() {
1602+
let (_dir, locator) = locator_with_tempdir();
1603+
let secret = Secret::SeedPhrase {
1604+
seed_phrase: TEST_SEED_PHRASE.to_string(),
1605+
hd_path: None,
1606+
};
1607+
locator.write_identity("alice", &secret).unwrap();
1608+
1609+
// The account derived at index 5 is only found when the lookup uses
1610+
// the same hd_path; the default (index 0) path must not match it.
1611+
let at_five = secret.public_key(Some(5)).unwrap();
1612+
assert!(locator
1613+
.secret_by_public_key(&at_five, Some(5))
1614+
.unwrap()
1615+
.is_some());
1616+
assert!(locator
1617+
.secret_by_public_key(&at_five, None)
1618+
.unwrap()
1619+
.is_none());
1620+
}
1621+
}
15281622
}

‎cmd/soroban-cli/src/config/mod.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ impl Args {
101101
}
102102

103103
pub fn key_pair(&self) -> Result<ed25519_dalek::SigningKey, Error> {
104-
let key = &self.source_account.resolve_secret(&self.locator)?;
104+
let key = &self
105+
.source_account
106+
.resolve_secret(&self.locator, self.hd_path())?;
105107
Ok(key.key_pair(self.hd_path())?)
106108
}
107109

0 commit comments

Comments
 (0)