Merge pull request #3 from narrrl/fix/address-keys-inactive-tokens

fix(account): open address tokens with inactive account keys, skip a broken address
This commit is contained in:
nvrl authored and GitHub committed 2026-10-31 13:33:22 +01:00
commit e403894b28
7 files changed
+307 -36

No files matched your search

+25
View File
@@ -7,6 +7,31 @@ commits a change ports are recorded in [UPSTREAM_SYNC.md](UPSTREAM_SYNC.md).
This file starts after 0.9.2; earlier changes are in the git history. This file starts after 0.9.2; earlier changes are in the git history.
## [0.10.1] — 2026-10-31
### Fixed
- **Address keys whose token is for an inactive account key.** An address key's
token was decrypted with the active account keys only. A token encrypted to
an account key since marked inactive never opened, although that key still
opens with the password, so the address failed with "has no primary key".
Tokens are now tried with every account key that opens, active ones first.
Signing still uses active keys only. `AccountClient::key_report` follows the
same chain.
- **One broken address no longer fails the account.** `AccountClient::addresses`
failed as a whole when any one address's keys did not open. That address is
now left out with a warning, and the others work. It fails only when no
address opens. `address_private_keys` names the reason for a left-out
address, and `default_address` fails with it when the left-out address is the
default, instead of handing out the next address.
### Added
- `AccountClient::address(id)`: one address by id, failing with the reason when
its keys did not open. `proton-drive-rs` uses it to resolve membership
addresses, which before said "membership address not found" for such an
address.
## [0.10.0] — 2026-10-26 ## [0.10.0] — 2026-10-26
### Added ### Added
Generated
+2 -2
View File
@@ -2060,7 +2060,7 @@ dependencies = [
[[package]] [[package]]
name = "proton-drive-rs" name = "proton-drive-rs"
version = "0.10.0" version = "0.10.1"
dependencies = [ dependencies = [
"async-trait", "async-trait",
"base64 0.23.1", "base64 0.23.1",
@@ -2085,7 +2085,7 @@ dependencies = [
[[package]] [[package]]
name = "proton-sdk" name = "proton-sdk"
version = "0.10.0" version = "0.10.1"
dependencies = [ dependencies = [
"aes-gcm 0.11.1", "aes-gcm 0.11.1",
"async-trait", "async-trait",
+1 -1
View File
@@ -3,7 +3,7 @@ resolver = "2"
members = ["crates/proton-sdk", "crates/proton-drive-rs"] members = ["crates/proton-sdk", "crates/proton-drive-rs"]
[workspace.package] [workspace.package]
version = "0.10.0" version = "0.10.1"
edition = "2024" edition = "2024"
license = "MIT" license = "MIT"
repository = "https://github.com/narrrl/proton-sdk-rs" repository = "https://github.com/narrrl/proton-sdk-rs"
+1 -1
View File
@@ -14,7 +14,7 @@ readme = "../../README.md"
documentation = "https://docs.rs/proton-drive-rs" documentation = "https://docs.rs/proton-drive-rs"
[dependencies] [dependencies]
proton-sdk = { path = "../proton-sdk", version = "0.10.0" } proton-sdk = { path = "../proton-sdk", version = "0.10.1" }
tokio.workspace = true tokio.workspace = true
reqwest.workspace = true reqwest.workspace = true
serde.workspace = true serde.workspace = true
+3 -14
View File
@@ -4737,13 +4737,7 @@ impl ProtonDriveClient {
/// The `AddressKeyID` of an address's primary key (write requests that bind /// The `AddressKeyID` of an address's primary key (write requests that bind
/// material to an address key need it alongside the `AddressID`). /// material to an address key need it alongside the `AddressID`).
async fn address_primary_key_id(&self, address_id: &AddressId) -> Result<AddressKeyId> { async fn address_primary_key_id(&self, address_id: &AddressId) -> Result<AddressKeyId> {
self.account Ok(self.account.address(address_id).await?.primary_key_id)
.addresses()
.await?
.into_iter()
.find(|address| &address.id == address_id)
.map(|address| address.primary_key_id)
.ok_or_else(|| ProtonError::invalid_operation("membership address not found"))
} }
/// Create a fresh-file draft: generate a node key + content key, encrypt the /// Create a fresh-file draft: generate a node key + content key, encrypt the
@@ -5473,13 +5467,8 @@ impl ProtonDriveClient {
&self, &self,
address_id: AddressId, address_id: AddressId,
) -> Result<(AddressId, String, PrivateKey)> { ) -> Result<(AddressId, String, PrivateKey)> {
let address = self // `address` names why an address whose keys did not open is missing.
.account let address = self.account.address(&address_id).await?;
.addresses()
.await?
.into_iter()
.find(|a| a.id == address_id)
.ok_or_else(|| ProtonError::invalid_operation("membership address not found"))?;
let keys = self.account.address_private_keys(&address_id).await?; let keys = self.account.address_private_keys(&address_id).await?;
let signing_key = keys let signing_key = keys
+263 -16
View File
@@ -97,8 +97,16 @@ struct Cache {
/// key id → passphrase derived from the mailbox password and key salts. /// key id → passphrase derived from the mailbox password and key salts.
key_passphrases: Option<KeyPassphrases>, key_passphrases: Option<KeyPassphrases>,
user_keys: Option<Vec<PrivateKey>>, user_keys: Option<Vec<PrivateKey>>,
/// Every user key that opens, active ones first: what an address key's
/// token may be encrypted to. See [`AccountClient::token_keys`].
token_keys: Option<Vec<PrivateKey>>,
addresses: Option<Vec<Address>>, addresses: Option<Vec<Address>>,
address_keys: HashMap<AddressId, Vec<PrivateKey>>, address_keys: HashMap<AddressId, Vec<PrivateKey>>,
/// Addresses whose keys did not open, with why. They are left out of
/// `addresses` so the others still work.
failed_addresses: HashMap<AddressId, String>,
/// Why the lowest-ordered address did not open, when it did not.
default_failed: Option<String>,
/// email → active (non-compromised) public keys, for authorship verification. /// email → active (non-compromised) public keys, for authorship verification.
public_keys: HashMap<String, Vec<PublicKey>>, public_keys: HashMap<String, Vec<PublicKey>>,
} }
@@ -193,8 +201,22 @@ impl AccountClient {
let response: AddressListResponse = self.inner.http.get("core/v4/addresses").await?; let response: AddressListResponse = self.inner.http.get("core/v4/addresses").await?;
let mut addresses = Vec::with_capacity(response.addresses.len()); let mut addresses = Vec::with_capacity(response.addresses.len());
let mut failed = HashMap::new();
let mut first_error = None;
for dto in &response.addresses { for dto in &response.addresses {
let (address, keys) = self.decrypt_address(dto).await?; // An address whose keys do not open is left out rather than
// failing the account: the shares on every other address still
// work, and one broken address must not stop them all.
let (address, keys) = match self.decrypt_address(dto).await {
Ok(decrypted) => decrypted,
Err(ProtonError::InvalidOperation(reason)) => {
tracing::warn!(address_id = %dto.id, %reason, "address keys do not open; skipping the address");
failed.insert(dto.id.clone(), reason.clone());
first_error.get_or_insert(reason);
continue;
}
Err(e) => return Err(e),
};
self.inner self.inner
.cache .cache
.lock() .lock()
@@ -203,16 +225,50 @@ impl AccountClient {
.insert(address.id.clone(), keys); .insert(address.id.clone(), keys);
addresses.push(address); addresses.push(address);
} }
if addresses.is_empty()
&& let Some(reason) = first_error
{
return Err(ProtonError::invalid_operation(reason));
}
addresses.sort_by_key(|a| a.order); addresses.sort_by_key(|a| a.order);
let default_failed = response
.addresses
.iter()
.min_by_key(|dto| dto.order)
.and_then(|dto| failed.get(&dto.id).cloned());
self.inner.cache.lock().await.addresses = Some(addresses.clone()); let mut cache = self.inner.cache.lock().await;
cache.failed_addresses = failed;
cache.default_failed = default_failed;
cache.addresses = Some(addresses.clone());
Ok(addresses) Ok(addresses)
} }
/// One address by id. An address whose keys did not open fails with the
/// reason, not as unknown.
pub async fn address(&self, address_id: &AddressId) -> Result<Address> {
let addresses = self.addresses().await?;
if let Some(address) = addresses.into_iter().find(|a| &a.id == address_id) {
return Ok(address);
}
let cache = self.inner.cache.lock().await;
Err(match cache.failed_addresses.get(address_id) {
Some(reason) => ProtonError::invalid_operation(reason.clone()),
None => ProtonError::invalid_operation(format!("no address {address_id}")),
})
}
/// The default (lowest-ordered) address. /// The default (lowest-ordered) address.
///
/// When that address's keys did not open, this fails with the reason
/// rather than handing out the next address: what it signs would then be
/// signed by an address the user did not choose.
pub async fn default_address(&self) -> Result<Address> { pub async fn default_address(&self) -> Result<Address> {
self.addresses() let addresses = self.addresses().await?;
.await? if let Some(reason) = self.inner.cache.lock().await.default_failed.clone() {
return Err(ProtonError::invalid_operation(reason));
}
addresses
.into_iter() .into_iter()
.next() .next()
.ok_or_else(|| ProtonError::invalid_operation("user has no address")) .ok_or_else(|| ProtonError::invalid_operation("user has no address"))
@@ -231,8 +287,12 @@ impl AccountClient {
self.addresses().await?; self.addresses().await?;
let cache = self.inner.cache.lock().await; let cache = self.inner.cache.lock().await;
cache.address_keys.get(address_id).cloned().ok_or_else(|| { if let Some(keys) = cache.address_keys.get(address_id) {
ProtonError::invalid_operation(format!("no keys for address {address_id}")) return Ok(keys.clone());
}
Err(match cache.failed_addresses.get(address_id) {
Some(reason) => ProtonError::invalid_operation(reason.clone()),
None => ProtonError::invalid_operation(format!("no keys for address {address_id}")),
}) })
} }
@@ -316,7 +376,7 @@ impl AccountClient {
self.user_keys().await.map(drop) self.user_keys().await.map(drop)
} }
/// Decrypted user (account) keys. /// Decrypted active user (account) keys.
async fn user_keys(&self) -> Result<Vec<PrivateKey>> { async fn user_keys(&self) -> Result<Vec<PrivateKey>> {
{ {
let cache = self.inner.cache.lock().await; let cache = self.inner.cache.lock().await;
@@ -329,16 +389,22 @@ impl AccountClient {
let response: UserResponse = self.inner.http.get("core/v4/users").await?; let response: UserResponse = self.inner.http.get("core/v4/users").await?;
let mut keys = Vec::new(); let mut keys = Vec::new();
let mut inactive = Vec::new();
for key_dto in &response.user.keys { for key_dto in &response.user.keys {
if !key_dto.is_active() {
continue;
}
let Some(passphrase) = passphrases.get(key_dto.id.as_str()) else { let Some(passphrase) = passphrases.get(key_dto.id.as_str()) else {
tracing::warn!(key_id = %key_dto.id, "no passphrase for user key"); if key_dto.is_active() {
tracing::warn!(key_id = %key_dto.id, "no passphrase for user key");
}
continue; continue;
}; };
match PrivateKey::from_armored(&key_dto.private_key, passphrase) { match PrivateKey::from_armored(&key_dto.private_key, passphrase) {
Ok(key) => keys.push(key), Ok(key) if key_dto.is_active() => keys.push(key),
Ok(key) => inactive.push(key),
// An inactive key is usually one whose passphrase went with an
// old password, so not opening is what it is expected to do.
Err(e) if !key_dto.is_active() => {
tracing::debug!(key_id = %key_dto.id, error = %e, "inactive user key does not open")
}
Err(e) => { Err(e) => {
tracing::warn!(key_id = %key_dto.id, error = %e, "failed to unlock user key") tracing::warn!(key_id = %key_dto.id, error = %e, "failed to unlock user key")
} }
@@ -356,10 +422,32 @@ impl AccountClient {
}); });
} }
self.inner.cache.lock().await.user_keys = Some(keys.clone()); let mut cache = self.inner.cache.lock().await;
cache.token_keys = Some(keys.iter().cloned().chain(inactive).collect());
cache.user_keys = Some(keys.clone());
Ok(keys) Ok(keys)
} }
/// Every user key that opens, active ones first, for decrypting address
/// key tokens.
///
/// A token can be encrypted to an account key that has since been marked
/// inactive, for example after a key was replaced. That key still opens
/// with the password, and the address key behind the token is still the
/// one the address's shares are encrypted to, so the token must be tried
/// with it too. Only active keys are used for anything else.
async fn token_keys(&self) -> Result<Vec<PrivateKey>> {
self.user_keys().await?;
Ok(self
.inner
.cache
.lock()
.await
.token_keys
.clone()
.unwrap_or_default())
}
/// The per-key passphrases, key id → passphrase, derived (and cached) from /// The per-key passphrases, key id → passphrase, derived (and cached) from
/// the mailbox password and the account's key salts. /// the mailbox password and the account's key salts.
/// ///
@@ -398,7 +486,7 @@ impl AccountClient {
/// Build the public [`Address`] and unlock all of its active private keys. /// Build the public [`Address`] and unlock all of its active private keys.
async fn decrypt_address(&self, dto: &AddressDto) -> Result<(Address, Vec<PrivateKey>)> { async fn decrypt_address(&self, dto: &AddressDto) -> Result<(Address, Vec<PrivateKey>)> {
let user_keys = self.user_keys().await?; let user_keys = self.token_keys().await?;
let passphrases = self.key_passphrases().await?; let passphrases = self.key_passphrases().await?;
let mut keys = Vec::new(); let mut keys = Vec::new();
@@ -672,8 +760,9 @@ mod tests {
assert!(report.user_keys.iter().all(|key| key.unlocks)); assert!(report.user_keys.iter().all(|key| key.unlocks));
let keys = &report.addresses[0].keys; let keys = &report.addresses[0].keys;
assert!(keys[0].unlocks && keys[0].error.is_none()); assert!(keys[0].unlocks && keys[0].error.is_none());
assert!(!keys[1].unlocks); // The report tries the token with every key that opens, as the unlock
assert!(keys[1].error.as_deref().unwrap().contains("missing key")); // path does.
assert!(keys[1].unlocks && keys[1].error.is_none());
let recipients = keys[1].token_recipients.as_ref().unwrap(); let recipients = keys[1].token_recipients.as_ref().unwrap();
let meant_for = report let meant_for = report
.user_key_named(recipients[0].as_deref().unwrap()) .user_key_named(recipients[0].as_deref().unwrap())
@@ -682,6 +771,164 @@ mod tests {
assert!(!meant_for.active); assert!(!meant_for.active);
} }
/// A body answering both `core/v4/users` and `core/v4/addresses`: one
/// active account key and one inactive one, and `addresses` as given.
fn account_with(
active: &crate::crypto::GeneratedNodeKey,
inactive: &crate::crypto::GeneratedNodeKey,
addresses: serde_json::Value,
) -> String {
serde_json::json!({
"Code": 1000,
"User": {
"ID": "user-id", "MaxSpace": 1, "UsedSpace": 0,
"Keys": [
{ "ID": "key-1", "PrivateKey": active.locked_armored, "Primary": 1, "Active": 1 },
{ "ID": "key-2", "PrivateKey": inactive.locked_armored, "Primary": 0, "Active": 0 }
]
},
"Addresses": addresses
})
.to_string()
}
/// An address key entry whose token, for `user_key`, carries the
/// passphrase of `address_key`.
fn address_key(
id: &str,
primary: bool,
user_key: &crate::crypto::GeneratedNodeKey,
address_key: &crate::crypto::GeneratedNodeKey,
) -> serde_json::Value {
let token = PrivateKey::from_armored(&user_key.locked_armored, &user_key.passphrase)
.unwrap()
.encrypt(&address_key.passphrase)
.unwrap();
serde_json::json!({
"ID": id, "PrivateKey": address_key.locked_armored, "Token": token,
"Signature": "signature", "Primary": i32::from(primary), "Active": 1
})
}
fn both_passphrases(
active: &crate::crypto::GeneratedNodeKey,
inactive: &crate::crypto::GeneratedNodeKey,
) -> KeyPassphrases {
KeyPassphrases::from_iter([
("key-1", active.passphrase.clone()),
("key-2", inactive.passphrase.clone()),
])
}
#[tokio::test]
async fn an_address_key_whose_token_is_for_an_inactive_account_key_opens() {
use crate::crypto::generate_node_key;
let (active, inactive) = (generate_node_key().unwrap(), generate_node_key().unwrap());
let (first, second) = (generate_node_key().unwrap(), generate_node_key().unwrap());
let body = account_with(
&active,
&inactive,
serde_json::json!([{
"ID": "address-1", "Email": "user@proton.me", "Status": 1, "Order": 1,
"Keys": [
address_key("address-key-1", true, &inactive, &first),
address_key("address-key-2", false, &active, &second)
]
}]),
);
let (session, _) = session_serving(body).await;
let client =
AccountClient::with_key_passphrases(&session, both_passphrases(&active, &inactive));
let address = client.default_address().await.expect("primary key opens");
assert_eq!(address.primary_key_id.as_str(), "address-key-1");
let keys = client.address_private_keys(&address.id).await.unwrap();
assert_eq!(keys.len(), 2);
}
#[tokio::test]
async fn an_address_whose_keys_do_not_open_leaves_the_others_working() {
use crate::crypto::generate_node_key;
let (active, inactive) = (generate_node_key().unwrap(), generate_node_key().unwrap());
let (good, broken, stranger) = (
generate_node_key().unwrap(),
generate_node_key().unwrap(),
generate_node_key().unwrap(),
);
// The broken address's only key has a token for a key not on the account.
let body = account_with(
&active,
&inactive,
serde_json::json!([
{ "ID": "address-broken", "Email": "old@proton.me", "Status": 1, "Order": 1,
"Keys": [address_key("address-key-b", true, &stranger, &broken)] },
{ "ID": "address-good", "Email": "user@proton.me", "Status": 1, "Order": 2,
"Keys": [address_key("address-key-g", true, &active, &good)] }
]),
);
let (session, _) = session_serving(body).await;
let client =
AccountClient::with_key_passphrases(&session, both_passphrases(&active, &inactive));
let addresses = client.addresses().await.expect("one address opens");
assert_eq!(addresses.len(), 1);
assert_eq!(addresses[0].id.as_str(), "address-good");
let good_id = addresses[0].id.clone();
assert_eq!(
client.address_private_keys(&good_id).await.unwrap().len(),
1
);
let broken_id: AddressId = "address-broken".to_owned().into();
for error in [
client
.address(&broken_id)
.await
.expect_err("broken")
.to_string(),
match client.address_private_keys(&broken_id).await {
Ok(_) => panic!("broken address opened"),
Err(e) => e.to_string(),
},
// The broken address is the default, so nothing is signed with
// another one in its place.
client
.default_address()
.await
.expect_err("default is broken")
.to_string(),
] {
assert!(error.contains("address-broken"), "{error}");
}
}
#[tokio::test]
async fn an_account_whose_addresses_all_fail_to_open_is_an_error() {
use crate::crypto::generate_node_key;
let (active, inactive) = (generate_node_key().unwrap(), generate_node_key().unwrap());
let (broken, stranger) = (generate_node_key().unwrap(), generate_node_key().unwrap());
let body = account_with(
&active,
&inactive,
serde_json::json!([{
"ID": "address-broken", "Email": "old@proton.me", "Status": 1, "Order": 1,
"Keys": [address_key("address-key-b", true, &stranger, &broken)]
}]),
);
let (session, _) = session_serving(body).await;
let client =
AccountClient::with_key_passphrases(&session, both_passphrases(&active, &inactive));
let error = client.addresses().await.expect_err("nothing opens");
assert!(
matches!(error, ProtonError::InvalidOperation(_)),
"{error:?}"
);
}
#[tokio::test] #[tokio::test]
async fn an_account_without_an_active_key_is_not_a_password_problem() { async fn an_account_without_an_active_key_is_not_a_password_problem() {
let key = crate::crypto::generate_node_key().unwrap(); let key = crate::crypto::generate_node_key().unwrap();
+12 -2
View File
@@ -26,7 +26,8 @@ pub struct KeyReport {
pub struct UserKeyReport { pub struct UserKeyReport {
pub id: UserKeyId, pub id: UserKeyId,
pub primary: bool, pub primary: bool,
/// `Active` from the server. The unlock path uses active keys only. /// `Active` from the server. The unlock path signs with active keys only,
/// but tries an inactive key that opens for address key tokens too.
pub active: bool, pub active: bool,
/// Whether the client's passphrase for this key opens it. Tried for /// Whether the client's passphrase for this key opens it. Tried for
/// inactive keys too, which the unlock path never does. /// inactive keys too, which the unlock path never does.
@@ -98,7 +99,10 @@ impl AccountClient {
let user: UserResponse = self.inner.http.get("core/v4/users").await?; let user: UserResponse = self.inner.http.get("core/v4/users").await?;
let mut user_keys = Vec::with_capacity(user.user.keys.len()); let mut user_keys = Vec::with_capacity(user.user.keys.len());
// Every key that opens, active first, as `AccountClient::token_keys`
// tries them for a token.
let mut active_unlocked = Vec::new(); let mut active_unlocked = Vec::new();
let mut inactive_unlocked = Vec::new();
for dto in &user.user.keys { for dto in &user.user.keys {
let unlocked = match passphrases.get(dto.id.as_str()) { let unlocked = match passphrases.get(dto.id.as_str()) {
Some(passphrase) => PrivateKey::from_armored(&dto.private_key, passphrase) Some(passphrase) => PrivateKey::from_armored(&dto.private_key, passphrase)
@@ -109,6 +113,8 @@ impl AccountClient {
Ok(key) => { Ok(key) => {
if dto.is_active() { if dto.is_active() {
active_unlocked.push(key); active_unlocked.push(key);
} else {
inactive_unlocked.push(key);
} }
(true, None) (true, None)
} }
@@ -123,6 +129,10 @@ impl AccountClient {
key_ids: crypto::key_ids(&dto.private_key).unwrap_or_default(), key_ids: crypto::key_ids(&dto.private_key).unwrap_or_default(),
}); });
} }
let token_keys: Vec<PrivateKey> = active_unlocked
.into_iter()
.chain(inactive_unlocked)
.collect();
let response: AddressListResponse = self.inner.http.get("core/v4/addresses").await?; let response: AddressListResponse = self.inner.http.get("core/v4/addresses").await?;
let mut addresses = Vec::with_capacity(response.addresses.len()); let mut addresses = Vec::with_capacity(response.addresses.len());
@@ -131,7 +141,7 @@ impl AccountClient {
for key_dto in &dto.keys { for key_dto in &dto.keys {
let passphrase = match (&key_dto.token, &key_dto.signature) { let passphrase = match (&key_dto.token, &key_dto.signature) {
(Some(token), Some(_signature)) => { (Some(token), Some(_signature)) => {
crypto::decrypt_armored_with_keys(token, &active_unlocked) crypto::decrypt_armored_with_keys(token, &token_keys)
.map_err(|e| format!("token: {e}")) .map_err(|e| format!("token: {e}"))
} }
_ => passphrases _ => passphrases