From a524b528b492530f1065bd5aa2443c3fa4117451 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Tue, 12 Aug 2025 14:01:42 +0100 Subject: [PATCH] Use AsyncFn with map_matching_items --- crates/uv-keyring/src/secret_service.rs | 383 +++++++++--------------- 1 file changed, 149 insertions(+), 234 deletions(-) diff --git a/crates/uv-keyring/src/secret_service.rs b/crates/uv-keyring/src/secret_service.rs index 8481427dc..64b35c29d 100644 --- a/crates/uv-keyring/src/secret_service.rs +++ b/crates/uv-keyring/src/secret_service.rs @@ -78,17 +78,18 @@ there is no "default" collection defined under WSL. So this keystore doesn't work "out of the box" on WSL. See the issue for more details and possible workarounds. */ + use std::collections::HashMap; use secret_service::{Collection, EncryptionType, Error, Item, SecretService}; -use crate::credential::{Credential, CredentialApi, CredentialBuilder, CredentialBuilderApi}; -use crate::error::{Error as ErrorCode, Result, decode_password}; +use super::credential::{Credential, CredentialApi, CredentialBuilder, CredentialBuilderApi}; +use super::error::{Error as ErrorCode, Result, decode_password}; /// The representation of an item in the secret-service. /// /// This structure has two roles. On the one hand, it captures all the -/// information a user specifies for an [`Entry`](crate::Entry) +/// information a user specifies for an [Entry](crate::Entry) /// and so is the basis for our search /// (or creation) of an item for that entry. On the other hand, when /// a search is ambiguous, each item found is represented by a credential that @@ -104,8 +105,9 @@ pub struct SsCredential { impl CredentialApi for SsCredential { /// Sets the password on a unique matching item, if it exists, or creates one if necessary. /// - /// If there are multiple matches, returns an [`Ambiguous`](ErrorCode::Ambiguous) error with - /// a credential for each matching item. + /// If there are multiple matches, + /// returns an [Ambiguous](ErrorCode::Ambiguous) error with a credential for each + /// matching item. /// /// When creating, the item is put into a collection named by the credential's `target` /// attribute. @@ -115,29 +117,26 @@ impl CredentialApi for SsCredential { /// Sets the secret on a unique matching item, if it exists, or creates one if necessary. /// - /// If there are multiple matches, returns an [`Ambiguous`](ErrorCode::Ambiguous) error - /// with a credential for each matching item. + /// If there are multiple matches, + /// returns an [Ambiguous](ErrorCode::Ambiguous) error with a credential for each + /// matching item. /// /// When creating, the item is put into a collection named by the credential's `target` /// attribute. async fn set_secret(&self, secret: &[u8]) -> Result<()> { // first try to find a unique, existing, matching item and set its password - let ss = SecretService::connect(EncryptionType::Dh) + let secret_vec = secret.to_vec(); + match self + .map_matching_items(async move |i| set_item_secret(i, &secret_vec).await, true) .await - .map_err(platform_failure)?; - match self.matching_items(&ss).await { - Ok(items) => { - for item in &items { - set_item_secret(item, secret).await?; - } - return Ok(()); - } + { + Ok(_) => return Ok(()), Err(ErrorCode::NoEntry) => {} Err(err) => return Err(err), } // if there is no existing item, create one for this credential. In order to create // an item, the credential must have an explicit target. All entries created with - // the [`new`] or [`new_with_target`] commands will have explicit targets. But entries + // the [new] or [new_with_target] commands will have explicit targets. But entries // created to wrap 3rd-party items that don't have `target` attributes may not. let ss = SecretService::connect(EncryptionType::Dh) .await @@ -162,96 +161,71 @@ impl CredentialApi for SsCredential { /// Gets the password on a unique matching item, if it exists. /// - /// If there are no matching items, returns a [`NoEntry`](ErrorCode::NoEntry) error. - /// If there are multiple matches, returns an [`Ambiguous`](ErrorCode::Ambiguous) + /// If there are no + /// matching items, returns a [NoEntry](ErrorCode::NoEntry) error. + /// If there are multiple matches, + /// returns an [Ambiguous](ErrorCode::Ambiguous) /// error with a credential for each matching item. async fn get_password(&self) -> Result { - let ss = SecretService::connect(EncryptionType::Dh) - .await - .map_err(platform_failure)?; - match self.unique_matching_item(&ss).await { - Ok(item) => get_item_password(&item).await, - Err(ErrorCode::NoEntry) => { - let collection = ss.get_default_collection().await.map_err(decode_error)?; - let item = self.find_unique_legacy_item(&collection).await?; - get_item_password(&item).await - } - Err(err) => Err(err), - } + Ok(self + .map_matching_items(get_item_password, true) + .await? + .remove(0)) } /// Gets the secret on a unique matching item, if it exists. /// /// If there are no - /// matching items, returns a [`NoEntry`](ErrorCode::NoEntry) error. + /// matching items, returns a [NoEntry](ErrorCode::NoEntry) error. /// If there are multiple matches, - /// returns an [`Ambiguous`](ErrorCode::Ambiguous) + /// returns an [Ambiguous](ErrorCode::Ambiguous) /// error with a credential for each matching item. async fn get_secret(&self) -> Result> { - let ss = SecretService::connect(EncryptionType::Dh) - .await - .map_err(platform_failure)?; - match self.unique_matching_item(&ss).await { - Ok(item) => get_item_secret(&item).await, - Err(ErrorCode::NoEntry) => { - let collection = ss.get_default_collection().await.map_err(decode_error)?; - let item = self.find_unique_legacy_item(&collection).await?; - get_item_secret(&item).await - } - Err(err) => Err(err), - } + Ok(self + .map_matching_items(get_item_secret, true) + .await? + .remove(0)) } /// Get attributes on a unique matching item, if it exists async fn get_attributes(&self) -> Result> { - let ss = SecretService::connect(EncryptionType::Dh) - .await - .map_err(platform_failure)?; - match self.unique_matching_item(&ss).await { - Ok(item) => get_item_attributes(&item).await, - Err(ErrorCode::NoEntry) => { - let collection = ss.get_default_collection().await.map_err(decode_error)?; - let item = self.find_unique_legacy_item(&collection).await?; - get_item_attributes(&item).await - } - Err(err) => Err(err), - } + let attributes: Vec> = + self.map_matching_items(get_item_attributes, true).await?; + Ok(attributes.into_iter().next().unwrap()) } /// Update attributes on a unique matching item, if it exists async fn update_attributes(&self, attributes: &HashMap<&str, &str>) -> Result<()> { - let ss = SecretService::connect(EncryptionType::Dh) - .await - .map_err(platform_failure)?; - match self.unique_matching_item(&ss).await { - Ok(item) => update_item_attributes(&item, attributes).await, - Err(ErrorCode::NoEntry) => { - let collection = ss.get_default_collection().await.map_err(decode_error)?; - let item = self.find_unique_legacy_item(&collection).await?; - update_item_attributes(&item, attributes).await - } - Err(err) => Err(err), - } + // Convert to owned data to avoid lifetime issues + let attributes_owned: HashMap = attributes + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + + self.map_matching_items( + async move |item| { + let attrs_ref: HashMap<&str, &str> = attributes_owned + .iter() + .map(|(k, v)| (k.as_str(), v.as_str())) + .collect(); + update_item_attributes(item, &attrs_ref).await + }, + true, + ) + .await?; + Ok(()) } /// Deletes the unique matching item, if it exists. /// - /// If there are no matching items, returns a [`NoEntry`](ErrorCode::NoEntry) error. - /// If there are multiple matches, returns an [`Ambiguous`](ErrorCode::Ambiguous) + /// If there are no + /// matching items, returns a [NoEntry](ErrorCode::NoEntry) error. + /// If there are multiple matches, + /// returns an [Ambiguous](ErrorCode::Ambiguous) /// error with a credential for each matching item. async fn delete_credential(&self) -> Result<()> { - let ss = SecretService::connect(EncryptionType::Dh) - .await - .map_err(platform_failure)?; - match self.unique_matching_item(&ss).await { - Ok(item) => delete_item(&item).await, - Err(ErrorCode::NoEntry) => { - let collection = ss.get_default_collection().await.map_err(decode_error)?; - let item = self.find_unique_legacy_item(&collection).await?; - delete_item(&item).await - } - Err(err) => Err(err), - } + self.map_matching_items(delete_item, true).await?; + Ok(()) } /// Return the underlying credential object with an `Any` type so that it can @@ -260,7 +234,7 @@ impl CredentialApi for SsCredential { self } - /// Expose the concrete debug formatter for use via the [`Credential`] trait + /// Expose the concrete debug formatter for use via the [Credential] trait fn debug_fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { std::fmt::Debug::fmt(self, f) } @@ -273,7 +247,7 @@ impl SsCredential { /// /// Creating this credential does not create a matching item. /// If there isn't already one there, it will be created only - /// when [`set_password`](SsCredential::set_password) is + /// when [set_password](SsCredential::set_password) is /// called. pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result { if let Some("") = target { @@ -290,7 +264,7 @@ impl SsCredential { Ok(Self { attributes, label: format!( - "{user}@{service}:{target} (uv v{})", + "{user}@{service}:{target} (keyring v{})", env!("CARGO_PKG_VERSION"), ), target: Some(target.to_string()), @@ -310,7 +284,7 @@ impl SsCredential { Ok(Self { attributes, label: format!( - "uv v{} for no target, service '{service}', user '{user}'", + "keyring-rs v{} for no target, service '{service}', user '{user}'", env!("CARGO_PKG_VERSION"), ), target: None, @@ -334,124 +308,80 @@ impl SsCredential { /// Construct a credential for this credential's underlying matching item, /// if there is exactly one. pub async fn new_from_matching_item(&self) -> Result { - let ss = SecretService::connect(EncryptionType::Dh) - .await - .map_err(platform_failure)?; - match self.unique_matching_item(&ss).await { - Ok(item) => Self::new_from_item(&item).await, - Err(ErrorCode::NoEntry) => { - let collection = ss.get_default_collection().await.map_err(decode_error)?; - let item = self.find_unique_legacy_item(&collection).await?; - Self::new_from_item(&item).await - } - Err(err) => Err(err), - } + Ok(self + .map_matching_items(Self::new_from_item, true) + .await? + .remove(0)) } /// If there are multiple matching items for this credential, get all of their passwords. /// - /// (This is useful if [`get_password`](SsCredential::get_password) - /// returns an [`Ambiguous`](ErrorCode::Ambiguous) error.) + /// (This is useful if [get_password](SsCredential::get_password) + /// returns an [Ambiguous](ErrorCode::Ambiguous) error.) pub async fn get_all_passwords(&self) -> Result> { - let ss = SecretService::connect(EncryptionType::Dh) - .await - .map_err(platform_failure)?; - match self.matching_items(&ss).await { - Ok(items) => { - let mut passwords = Vec::new(); - for item in &items { - passwords.push(get_item_password(item).await?); - } - Ok(passwords) - } - Err(ErrorCode::NoEntry) => { - let collection = ss.get_default_collection().await.map_err(decode_error)?; - let items = self.find_legacy_items(&collection).await?; - let mut passwords = Vec::new(); - for item in &items { - passwords.push(get_item_password(item).await?); - } - Ok(passwords) - } - Err(err) => Err(err), - } + self.map_matching_items(get_item_password, false).await } /// If there are multiple matching items for this credential, delete all of them. /// - /// (This is useful if [`delete_credential`](SsCredential::delete_credential) - /// returns an [`Ambiguous`](ErrorCode::Ambiguous) error.) + /// (This is useful if [delete_credential](SsCredential::delete_credential) + /// returns an [Ambiguous](ErrorCode::Ambiguous) error.) pub async fn delete_all_passwords(&self) -> Result<()> { + self.map_matching_items(delete_item, false).await?; + Ok(()) + } + + /// Map an async function over the items matching this credential. + /// + /// Items are unlocked before the function is applied. + /// + /// If `require_unique` is true, and there are no matching items, then + /// a [NoEntry](ErrorCode::NoEntry) error is returned. + /// If `require_unique` is true, and there are multiple matches, + /// then an [Ambiguous](ErrorCode::Ambiguous) error is returned + /// with a vector containing one + /// credential for each of the matching items. + async fn map_matching_items(&self, f: F, require_unique: bool) -> Result> + where + F: AsyncFn(&Item<'_>) -> Result, + T: Sized, + { let ss = SecretService::connect(EncryptionType::Dh) .await .map_err(platform_failure)?; - match self.matching_items(&ss).await { - Ok(items) => { - for item in &items { - delete_item(item).await?; - } - Ok(()) - } - Err(ErrorCode::NoEntry) => { - let collection = ss.get_default_collection().await.map_err(decode_error)?; - let items = self.find_legacy_items(&collection).await?; - for item in &items { - delete_item(item).await?; - } - Ok(()) - } - Err(err) => Err(err), - } - } - - /// Find and unlock items matching this credential. - /// - /// Items are unlocked before being returned. - async fn matching_items<'a>(&self, ss: &'a SecretService<'_>) -> Result>> { let attributes: HashMap<&str, &str> = self.search_attributes(false).into_iter().collect(); let search = ss.search_items(attributes).await.map_err(decode_error)?; let count = search.locked.len() + search.unlocked.len(); if count == 0 { - return Err(ErrorCode::NoEntry); + if let Some("default") = self.target.as_deref() { + return self.map_matching_legacy_items(&ss, f, require_unique).await; + } } - - let mut results: Vec> = vec![]; - for item in search.unlocked { - results.push(item); + if require_unique { + if count == 0 { + return Err(ErrorCode::NoEntry); + } else if count > 1 { + let mut creds: Vec> = vec![]; + for item in search.locked.iter().chain(search.unlocked.iter()) { + let cred = Self::new_from_item(item).await?; + creds.push(Box::new(cred)) + } + return Err(ErrorCode::Ambiguous(creds)); + } } - for item in search.locked { + let mut results: Vec = vec![]; + for item in search.unlocked.iter() { + results.push(f(item).await?); + } + for item in search.locked.iter() { item.unlock().await.map_err(decode_error)?; - results.push(item); + results.push(f(item).await?); } Ok(results) } - /// Find and unlock a unique item matching this credential. - /// - /// If there are no matching items, then a [`NoEntry`](ErrorCode::NoEntry) error is returned. - /// If there are multiple matches, then an [`Ambiguous`](ErrorCode::Ambiguous) error is - /// returned with a vector containing one credential for each of the matching items. - async fn unique_matching_item<'a>(&self, ss: &'a SecretService<'_>) -> Result> { - let mut items = self.matching_items(ss).await?; - match items.len() { - 0 => Err(ErrorCode::NoEntry), - 1 => Ok(items.pop().unwrap()), - _ => { - let mut creds: Vec> = vec![]; - let attributes: HashMap<&str, &str> = - self.search_attributes(false).into_iter().collect(); - let search = ss.search_items(attributes).await.map_err(decode_error)?; - for item in search.locked.iter().chain(search.unlocked.iter()) { - let cred = Self::new_from_item(item).await?; - creds.push(Box::new(cred)); - } - Err(ErrorCode::Ambiguous(creds)) - } - } - } - - /// Find legacy items in the given collection if the credential being searched has - /// the default target. + /// Map an async function over items that older versions of keyring + /// would have matched against this credential. /// /// Keyring v1 created secret service items that had no target attribute, and it was /// only able to create items in the default collection. Keyring v2, and Keyring v3.1, @@ -469,59 +399,43 @@ impl SsCredential { /// collection. /// /// So with keyring v3.2.1, if the service-wide search fails to find any matching - /// credential, and the credential being searched for has the default target, - /// we fall back and search the default collection for a v1-style credential. + /// credential, and the credential being searched for has the default target (or + /// no target), we fall back and search the default collection for a v1-style credential. /// That preserves the legacy behavior at the cost of a second round-trip through /// the secret service for the collection search. - /// - /// Returns a [`NoEntry`](ErrorCode::NoEntry) error if there are no matching items or - /// if the credential being searched does not have the default target. - async fn find_legacy_items<'a>(&self, collection: &'a Collection<'_>) -> Result>> { - if let Some("default") = self.target.as_deref() { - let attributes = self.search_attributes(true); - let search = collection - .search_items(attributes) - .await - .map_err(decode_error)?; - if search.is_empty() { - return Err(ErrorCode::NoEntry); - } - - Ok(search) - } else { - Err(ErrorCode::NoEntry) - } - } - - /// Find unique legacy item in the given collection if the credential being searched has - /// the default target. - /// - /// Returns a [`NoEntry`](ErrorCode::NoEntry) error if there are no matching items or - /// if the credential being searched does not have the default target. - /// If there are multiple matches, then an [`Ambiguous`](ErrorCode::Ambiguous) error is - /// returned with a vector containing one credential for each of the matching items. - async fn find_unique_legacy_item<'a>( + pub async fn map_matching_legacy_items( &self, - collection: &'a Collection<'_>, - ) -> Result> { - let mut items = self.find_legacy_items(collection).await?; - match items.len() { - 0 => Err(ErrorCode::NoEntry), - 1 => Ok(items.pop().unwrap()), - _ => { + ss: &SecretService<'_>, + f: F, + require_unique: bool, + ) -> Result> + where + F: AsyncFn(&Item<'_>) -> Result, + T: Sized, + { + let collection = ss.get_default_collection().await.map_err(decode_error)?; + let attributes = self.search_attributes(true); + let search = collection + .search_items(attributes) + .await + .map_err(decode_error)?; + if require_unique { + if search.is_empty() && require_unique { + return Err(ErrorCode::NoEntry); + } else if search.len() > 1 { let mut creds: Vec> = vec![]; - let attributes = self.search_attributes(true); - let search = collection - .search_items(attributes) - .await - .map_err(decode_error)?; - for item in &search { + for item in search.iter() { let cred = Self::new_from_item(item).await?; - creds.push(Box::new(cred)); + creds.push(Box::new(cred)) } - Err(ErrorCode::Ambiguous(creds)) + return Err(ErrorCode::Ambiguous(creds)); } } + let mut results: Vec = vec![]; + for item in search.iter() { + results.push(f(item).await?); + } + Ok(results) } /// Using strings in the credential map makes managing the lifetime @@ -535,7 +449,7 @@ impl SsCredential { .collect() } - /// Similar to [`all_attributes`](SsCredential::all_attributes), + /// Similar to [all_attributes](SsCredential::all_attributes), /// but this just selects the ones we search on fn search_attributes(&self, omit_target: bool) -> HashMap<&str, &str> { let mut result: HashMap<&str, &str> = HashMap::new(); @@ -550,7 +464,7 @@ impl SsCredential { /// The builder for secret-service credentials #[derive(Debug, Default)] -pub struct SsCredentialBuilder; +pub struct SsCredentialBuilder {} /// Returns an instance of the secret-service credential builder. /// @@ -561,7 +475,7 @@ pub fn default_credential_builder() -> Box { } impl CredentialBuilderApi for SsCredentialBuilder { - /// Build an [`SsCredential`] for the given target, service, and user. + /// Build an [SsCredential] for the given target, service, and user. fn build(&self, target: Option<&str>, service: &str, user: &str) -> Result> { Ok(Box::new(SsCredential::new_with_target( target, service, user, @@ -569,7 +483,7 @@ impl CredentialBuilderApi for SsCredentialBuilder { } /// Return the underlying builder object with an `Any` type so that it can - /// be downgraded to an [`SsCredentialBuilder`] for platform-specific processing. + /// be downgraded to an [SsCredentialBuilder] for platform-specific processing. fn as_any(&self) -> &dyn std::any::Any { self } @@ -590,8 +504,7 @@ pub async fn get_collection<'a>(ss: &'a SecretService<'_>, name: &str) -> Result let all = ss.get_all_collections().await.map_err(decode_error)?; let mut found = None; for c in all { - let label = c.get_label().await.map_err(decode_error)?; - if label.eq(name) { + if c.get_label().await.map_err(decode_error)?.eq(name) { found = Some(c); break; } @@ -660,10 +573,10 @@ pub async fn update_item_attributes( ) -> Result<()> { let existing = item.get_attributes().await.map_err(decode_error)?; let mut updated: HashMap<&str, &str> = HashMap::new(); - for (k, v) in &existing { + for (k, v) in existing.iter() { updated.insert(k, v); } - for (k, v) in attributes { + for (k, v) in attributes.iter() { if k.eq(&"target") || k.eq(&"service") || k.eq(&"username") { continue; } @@ -987,6 +900,7 @@ mod tests { async fn delete_collection(name: &str) { let ss = SecretService::connect(EncryptionType::Dh) + .await .await .expect("Can't connect to secret service"); let collection = super::get_collection(&ss, name) @@ -1001,6 +915,7 @@ mod tests { let cred = SsCredential::new_with_no_target(name, name) .expect("Can't create credential with no target"); let ss = SecretService::connect(EncryptionType::Dh) + .await .await .expect("Can't connect to secret service"); let collection = ss