diff --git a/Cargo.lock b/Cargo.lock index 088c44fe8..f1fa0d284 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3540,37 +3540,6 @@ version = "0.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c20b6793b5c2fa6553b250154b78d6d0db37e72700ae35fad9387a46f487c97" -[[package]] -name = "rpassword" -version = "7.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "66d4c8b64f049c6721ec8ccec37ddfc3d641c4a7fca57e8f2a89de509c73df39" -dependencies = [ - "libc", - "rtoolbox", - "windows-sys 0.59.0", -] - -[[package]] -name = "rprompt" -version = "2.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69abf524bb9ccb7c071f7231441288d74b48d176cb309eb00e6f77d186c6e035" -dependencies = [ - "rtoolbox", - "windows-sys 0.59.0", -] - -[[package]] -name = "rtoolbox" -version = "0.0.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a7cc970b249fbe527d6e02e0a227762c9108b2f49d81094fe357ffc6d14d7f6f" -dependencies = [ - "libc", - "windows-sys 0.52.0", -] - [[package]] name = "rust-netrc" version = "0.1.2" @@ -5790,19 +5759,13 @@ name = "uv-keyring" version = "0.0.1" dependencies = [ "async-trait", - "base64 0.22.1", "byteorder", - "clap", "doc-comment", "env_logger", "fastrand", - "log", - "rpassword", - "rprompt", "secret-service", "security-framework", "tokio", - "whoami", "windows-sys 0.59.0", "zeroize", ] diff --git a/crates/uv-keyring/Cargo.toml b/crates/uv-keyring/Cargo.toml index 605664b94..909b06d59 100644 --- a/crates/uv-keyring/Cargo.toml +++ b/crates/uv-keyring/Cargo.toml @@ -11,8 +11,9 @@ workspace = true [features] default = ["apple-native", "secret-service", "windows-native"] +keyring-tests = [] -## Use the built-in Keychain Services on macOS and iOS +## Use the built-in Keychain Services on macOS apple-native = ["dep:security-framework"] ## Use the secret-service on *nix. secret-service = ["dep:secret-service"] @@ -21,10 +22,9 @@ windows-native = ["dep:windows-sys", "dep:byteorder"] [dependencies] async-trait = { workspace = true } -log = "0.4" tokio = { workspace = true } -[target.'cfg(any(target_os = "macos", target_os = "ios"))'.dependencies] +[target.'cfg(target_os = "macos")'.dependencies] security-framework = { version = "3", optional = true } [target.'cfg(any(target_os = "linux",target_os = "freebsd", target_os = "openbsd"))'.dependencies] @@ -35,26 +35,11 @@ byteorder = { version = "1", optional = true } windows-sys = { version = "0.59", features = ["Win32_Foundation", "Win32_Security_Credentials"], optional = true } zeroize = "1.8.1" -[[example]] -name = "iostest" -path = "examples/ios.rs" -crate-type = ["staticlib"] - -[[example]] -name = "keyring-cli" -path = "examples/cli.rs" - [dev-dependencies] -base64 = { workspace = true } -clap = { workspace = true } doc-comment = "0.3" env_logger = "0.11.5" fastrand = "2" -rpassword = "7" -rprompt = "2" -tokio = { workspace = true } -whoami = "1.5" [package.metadata.docs.rs] default-target = "x86_64-unknown-linux-gnu" -targets = ["x86_64-unknown-linux-gnu", "aarch64-apple-darwin", "aarch64-apple-ios", "x86_64-pc-windows-msvc"] +targets = ["x86_64-unknown-linux-gnu", "aarch64-apple-darwin", "x86_64-pc-windows-msvc"] diff --git a/crates/uv-keyring/README.md b/crates/uv-keyring/README.md index cd379d773..613bff88a 100644 --- a/crates/uv-keyring/README.md +++ b/crates/uv-keyring/README.md @@ -1,14 +1,22 @@ # uv-keyring -This is vendored from [keyring-rs crate](https://github.com/open-source-cooperative/keyring-rs) commit 9635a2f53a19eb7f188cdc4e38982dcb19caee00. +This is vendored from [keyring-rs crate](https://github.com/open-source-cooperative/keyring-rs) +commit 9635a2f53a19eb7f188cdc4e38982dcb19caee00. -A cross-platform library to manage storage and retrieval of passwords (and other secrets) in the underlying platform secure store, with a fully-developed example that provides a command-line interface. +A cross-platform library to manage storage and retrieval of passwords (and other secrets) in the +underlying platform secure store, with a fully-developed example that provides a command-line +interface. ## Usage -You can use the `Entry::new` function to create a new keyring entry. The `new` function takes a service name and a user's name which together identify the entry. +You can use the `Entry::new` function to create a new keyring entry. The `new` function takes a +service name and a user's name which together identify the entry. -Passwords (strings) or secrets (binary data) can be added to an entry using its `set_password` or `set_secret` methods, respectively. (These methods create or update an entry in the underlying platform's persistent credential store.) The password or secret can then be read back using the `get_password` or `get_secret` methods. The underlying credential (with its password/secret data) can then be removed using the `delete_credential` method. +Passwords (strings) or secrets (binary data) can be added to an entry using its `set_password` or +`set_secret` methods, respectively. (These methods create or update an entry in the underlying +platform's persistent credential store.) The password or secret can then be read back using the +`get_password` or `get_secret` methods. The underlying credential (with its password/secret data) +can then be removed using the `delete_credential` method. ```rust use keyring::{Entry, Result}; @@ -25,56 +33,32 @@ fn main() -> Result<()> { ## Errors -Creating and operating on entries can yield a `keyring::Error` which provides both a platform-independent code that classifies the error and, where relevant, underlying platform errors or more information about what went wrong. - -## Examples - -The keychain-rs project contains a sample application (`keyring-cli`) and a sample library (`ios`). - -The `keyring-cli` application is a command-line interface to the full functionality of the keyring. Invoke it without arguments to see usage information. It handles binary data input and output using base64 encoding. It can be installed using `cargo install` and used to experiment with library functionality. It can also be used when debugging keyring-based applications to probe the contents of the credential store. - -The `ios` library is a full exercise of all the iOS functionality; it's meant to be loaded into an iOS test harness such as the one found in [this project](https://github.com/brotskydotcom/rust-on-ios). - -## Client Testing - -This crate comes with a mock credential store that can be used by clients who want to test without accessing the native platform store. The mock store is cross-platform and allows mocking errors as well as successes. See the [developer docs](https://docs.rs/keyring/) for details. - -## Extensibility - -This crate allows clients to bring their own credential store by providing traits that clients can implement. See the [developer docs](https://docs.rs/keyring/) for details. +Creating and operating on entries can yield a `keyring::Error` which provides both a +platform-independent code that classifies the error and, where relevant, underlying platform errors +or more information about what went wrong. ## Platforms This crate provides built-in implementations of the following platform-specific credential stores: -* _Linux_, _FreeBSD_, _OpenBSD_: The DBus-based Secret Service. -* _macOS_, _iOS_: Keychain Services. -* _Windows_: The Windows Credential Manager. +- _Linux_, _FreeBSD_, _OpenBSD_: The DBus-based Secret Service. +- _macOS_: Keychain Services. +- _Windows_: The Windows Credential Manager. -It can be built and used on other platforms, but will not provide a built-in credential store implementation; you will have to bring your own. +It can be built and used on other platforms, but will not provide a built-in credential store +implementation; you will have to bring your own. ### Platform-specific issues -Since neither the maintainers nor GitHub do testing on BSD variants, we rely on contributors to support these platforms. Thanks for your help! +If you use the _Secret Service_ as your credential store, be aware that every call to the Secret +Service is done via an inter-process call, which takes time (typically tens if not hundreds of +milliseconds). -If you use the *Secret Service* as your credential store, be aware of the following: +If you use the _Windows-native credential store_, be careful about multi-threaded access, because +the Windows credential store does not guarantee your calls will be serialized in the order they are +made. Always access any single credential from just one thread at a time, and if you are doing +operations on multiple credentials that require a particular serialization order, perform all those +operations from the same thread. -* The default build of this crate expects that `libdbus` will be installed on users' machines. If you have users whose machines might not have `libdbus` installed, you can specify the `vendored` feature when building this crate to statically link the dbus library with your app. -* Every call to the Secret Service is done via an inter-process call, which takes time (typically tens if not hundreds of milliseconds). -* By default, this implementation does not encrypt secrets when sending them to or fetching them from the Dbus. If you want them encrypted, you can specify the `encrypted` feature when building this crate. - -If you use the *Windows-native credential store*, be careful about multi-threaded access, because the Windows credential store does not guarantee your calls will be serialized in the order they are made. Always access any single credential from just one thread at a time, and if you are doing operations on multiple credentials that require a particular serialization order, perform all those operations from the same thread. - -The *macOS and iOS credential stores* do not allow service names or usernames to be empty, because empty fields are treated as wildcards on lookup. Use some default, non-empty value instead. - -## Upgrading from v3 - -There are no functional API changes between v4 and v3. All the changes are in the keystore implementations and how features are used to select keystores: - -* Version 4 of this crate removes a number of the built-in credential stores that were available in version 3, namely the async secret service and linux keyutils. These keystores are being contributed directly to the existing [secret-service](https://crates.io/crates/secret-service) and [linux-keyutils](https://crates.io/crates/linux-keyutils) crates, respectively. -* Version 4 of this crate dispenses with the need to explicitly specify which credential store you want to use on each platform. Instead, the default feature set provides a single credential store on each platform. If you would rather bring your own store, and not build this crate's built-in ones, you can simply suppress the default feature set. -* The built-in macOS keystore now supports use of the Data Protection keychain, which is the same keychain used by iOS. You can specify a target of "Data Protection" (or simply "Protected") to write and read credentials in that keychain. - -All v2/v3 data is fully forward-compatible with v4 data; there have been no changes at all in that respect. - -The Rust edition of this crate has moved to 2024, and the MSRV has moved to 1.85. +The _macOS credential store_ does not allow service names or usernames to be empty, because empty +fields are treated as wildcards on lookup. Use some default, non-empty value instead. diff --git a/crates/uv-keyring/examples/cli.rs b/crates/uv-keyring/examples/cli.rs deleted file mode 100644 index dee84e450..000000000 --- a/crates/uv-keyring/examples/cli.rs +++ /dev/null @@ -1,336 +0,0 @@ -extern crate uv_keyring; - -use clap::{Args, Parser}; -use std::collections::HashMap; - -use uv_keyring::{Entry, Error, Result}; - -#[tokio::main(flavor = "current_thread")] -async fn main() { - let mut args: Cli = Cli::parse(); - if args.user.eq_ignore_ascii_case("") { - args.user = whoami::username() - } - let entry = match args.entry_for() { - Ok(entry) => entry, - Err(err) => { - if args.verbose { - let description = args.description(); - eprintln!("Couldn't create entry for '{description}': {err}") - } - std::process::exit(1) - } - }; - match &args.command { - Command::Set { .. } => { - let value = args.get_password_and_attributes(); - match &value { - Value::Secret(secret) => match entry.set_secret(secret).await { - Ok(()) => args.success_message_for(&value), - Err(err) => args.error_message_for(err), - }, - Value::Password(password) => match entry.set_password(password).await { - Ok(()) => args.success_message_for(&value), - Err(err) => args.error_message_for(err), - }, - Value::Attributes(attributes) => { - let attrs: HashMap<&str, &str> = attributes - .iter() - .map(|(k, v)| (k.as_str(), v.as_str())) - .collect(); - match entry.update_attributes(&attrs).await { - Ok(()) => args.success_message_for(&value), - Err(err) => args.error_message_for(err), - } - } - _ => panic!("Can't set without a value"), - } - } - Command::Password => match entry.get_password().await { - Ok(password) => { - println!("{password}"); - args.success_message_for(&Value::Password(password)); - } - Err(err) => args.error_message_for(err), - }, - Command::Secret => match entry.get_secret().await { - Ok(secret) => { - println!("{}", secret_string(&secret)); - args.success_message_for(&Value::Secret(secret.to_vec())); - } - Err(err) => args.error_message_for(err), - }, - Command::Attributes => match entry.get_attributes().await { - Ok(attributes) => { - println!("{}", attributes_string(&attributes)); - args.success_message_for(&Value::Attributes(attributes)); - } - Err(err) => args.error_message_for(err), - }, - Command::Delete => match entry.delete_credential().await { - Ok(()) => args.success_message_for(&Value::None), - Err(err) => args.error_message_for(err), - }, - } -} - -#[derive(Debug, Parser)] -#[clap(author = "github.com/hwchen/keyring-rs")] -/// Keyring CLI: A command-line interface to platform secure storage -pub struct Cli { - #[clap(short, long, action, verbatim_doc_comment)] - /// Write debugging info to stderr, including retrieved passwords and secrets. - /// If an operation fails, detailed error information is provided. - pub verbose: bool, - - #[clap(short, long, value_parser)] - /// The (optional) target for the entry. - pub target: Option, - - #[clap(short, long, value_parser, default_value = "keyring-cli")] - /// The service for the entry. - pub service: String, - - #[clap(short, long, value_parser, default_value = "")] - /// The user for the entry. - pub user: String, - - #[clap(subcommand)] - pub command: Command, -} - -#[derive(Debug, Parser)] -pub enum Command { - /// Set the password or update the attributes in the secure store - Set { - #[command(flatten)] - what: What, - - #[clap(value_parser)] - /// The input to parse. If not specified, it will be - /// read interactively from the terminal. Password/secret - /// input will not be echoed. - input: Option, - }, - /// Retrieve the (string) password from the secure store - /// and write it to the standard output. - Password, - /// Retrieve the (binary) secret from the secure store - /// and write it in base64 encoding to the standard output. - Secret, - /// Retrieve attributes available in the secure store. - Attributes, - /// Delete the credential from the secure store. - Delete, -} - -#[derive(Debug, Args)] -#[group(multiple = false, required = true)] -pub struct What { - #[clap(short, long, action, help = "The input is a password")] - password: bool, - - #[clap(short, long, action, help = "The input is a base64-encoded secret")] - secret: bool, - - #[clap( - short, - long, - action, - help = "The input is comma-separated, key=val attribute pairs" - )] - attributes: bool, -} - -enum Value { - Secret(Vec), - Password(String), - Attributes(HashMap), - None, -} - -impl Cli { - fn description(&self) -> String { - if let Some(target) = &self.target { - format!("{}@{}:{target}", &self.user, &self.service) - } else { - format!("{}@{}", &self.user, &self.service) - } - } - - fn entry_for(&self) -> Result { - if let Some(target) = &self.target { - Entry::new_with_target(target, &self.service, &self.user) - } else { - Entry::new(&self.service, &self.user) - } - } - - fn error_message_for(&self, err: Error) { - if self.verbose { - let description = self.description(); - match err { - Error::NoEntry => { - eprintln!("No credential found for '{description}'"); - } - Error::Ambiguous(creds) => { - eprintln!("More than one credential found for '{description}': {creds:?}"); - } - err => match self.command { - Command::Set { .. } => { - eprintln!("Couldn't set credential data for '{description}': {err}"); - } - Command::Password => { - eprintln!("Couldn't get password for '{description}': {err}"); - } - Command::Secret => { - eprintln!("Couldn't get secret for '{description}': {err}"); - } - Command::Attributes => { - eprintln!("Couldn't get attributes for '{description}': {err}"); - } - Command::Delete => { - eprintln!("Couldn't delete credential for '{description}': {err}"); - } - }, - } - } - std::process::exit(1) - } - - fn success_message_for(&self, value: &Value) { - if !self.verbose { - return; - } - let description = self.description(); - match self.command { - Command::Set { .. } => match value { - Value::Secret(secret) => { - let secret = secret_string(secret); - eprintln!("Set secret for '{description}' to decode of '{secret}'"); - } - Value::Password(password) => { - eprintln!("Set password for '{description}' to '{password}'"); - } - Value::Attributes(attributes) => { - eprintln!("The following attributes for '{description}' were sent for update:"); - eprint_attributes(attributes); - } - _ => panic!("Can't set without a value"), - }, - Command::Password => { - match value { - Value::Password(password) => { - eprintln!("Password for '{description}' is '{password}'"); - } - _ => panic!("Wrong value type for command"), - }; - } - Command::Secret => match value { - Value::Secret(secret) => { - let encoded = secret_string(secret); - eprintln!("Secret for '{description}' encodes as {encoded}"); - } - _ => panic!("Wrong value type for command"), - }, - Command::Attributes => match value { - Value::Attributes(attributes) => { - if attributes.is_empty() { - eprintln!("No attributes found for '{description}'"); - } else { - eprintln!("Attributes for '{description}' are:"); - eprint_attributes(attributes); - } - } - _ => panic!("Wrong value type for command"), - }, - Command::Delete => { - eprintln!("Successfully deleted credential for '{description}'"); - } - } - } - - fn get_password_and_attributes(&self) -> Value { - if let Command::Set { what, input } = &self.command { - if what.password { - Value::Password(read_password(input)) - } else if what.secret { - Value::Secret(decode_secret(input)) - } else { - Value::Attributes(parse_attributes(input)) - } - } else { - panic!("Can't happen: asking for password and attributes on non-set command") - } - } -} - -fn secret_string(secret: &[u8]) -> String { - use base64::prelude::*; - - BASE64_STANDARD.encode(secret) -} - -fn eprint_attributes(attributes: &HashMap) { - for (key, value) in attributes { - println!(" {key}: {value}"); - } -} - -fn decode_secret(input: &Option) -> Vec { - use base64::prelude::*; - - let encoded = if let Some(input) = input { - input.clone() - } else { - rpassword::prompt_password("Base64 encoding: ").unwrap_or_else(|_| String::new()) - }; - if encoded.is_empty() { - return Vec::new(); - } - match BASE64_STANDARD.decode(encoded) { - Ok(secret) => secret, - Err(err) => { - eprintln!("Sorry, the provided secret data is not base64-encoded: {err}"); - std::process::exit(1); - } - } -} - -fn read_password(input: &Option) -> String { - if let Some(input) = input { - input.clone() - } else { - rpassword::prompt_password("Password: ").unwrap_or_else(|_| String::new()) - } -} - -fn attributes_string(attributes: &HashMap) -> String { - let strings = attributes - .iter() - .map(|(k, v)| format!("{}={}", k, v)) - .collect::>(); - strings.join(",") -} - -fn parse_attributes(input: &Option) -> HashMap { - let input = if let Some(input) = input { - input.clone() - } else { - rprompt::prompt_reply("Attributes: ").unwrap_or_else(|_| String::new()) - }; - if input.is_empty() { - eprintln!("You must specify at least one key=value attribute pair to set") - } - let mut attributes = HashMap::new(); - let parts = input.split(','); - for s in parts.into_iter() { - let parts: Vec<&str> = s.split("=").collect(); - if parts.len() != 2 || parts[0].is_empty() { - eprintln!("Sorry, this part of the attributes string is not a key=val pair: {s}"); - std::process::exit(1); - } - attributes.insert(parts[0].to_string(), parts[1].to_string()); - } - attributes -} diff --git a/crates/uv-keyring/examples/ios.rs b/crates/uv-keyring/examples/ios.rs deleted file mode 100644 index 72b6329b6..000000000 --- a/crates/uv-keyring/examples/ios.rs +++ /dev/null @@ -1,101 +0,0 @@ -use uv_keyring::{Entry, Error}; - -#[unsafe(no_mangle)] -extern "C" fn test() { - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .unwrap(); - runtime.block_on(async { - test_invalid_parameter(); - test_empty_keyring().await; - test_empty_password_input().await; - test_round_trip_ascii_password().await; - test_round_trip_non_ascii_password().await; - test_update_password().await; - }); -} - -fn test_invalid_parameter() { - let entry = Entry::new("", "user"); - assert!( - matches!(entry, Err(Error::Invalid(_, _))), - "Created entry with empty service" - ); - let entry = Entry::new("service", ""); - assert!( - matches!(entry, Err(Error::Invalid(_, _))), - "Created entry with empty user" - ); - let entry = Entry::new_with_target("test", "service", "user"); - assert!( - matches!(entry, Err(Error::Invalid(_, _))), - "Created entry with non-default target" - ); -} - -async fn test_empty_keyring() { - let name = "test_empty_keyring".to_string(); - let entry = Entry::new(&name, &name).expect("Failed to create entry"); - assert!(matches!(entry.get_password().await, Err(Error::NoEntry))) -} - -async fn test_empty_password_input() { - let name = "test_empty_password_input".to_string(); - let entry = Entry::new(&name, &name).expect("Failed to create entry"); - let in_pass = ""; - entry - .set_password(in_pass) - .await - .expect("Couldn't set empty password"); - let out_pass = entry - .get_password() - .await - .expect("Couldn't get empty password"); - assert_eq!(in_pass, out_pass); - entry - .delete_credential() - .await - .expect("Couldn't delete credential with empty password"); - assert!( - matches!(entry.get_password().await, Err(Error::NoEntry)), - "Able to read a deleted password" - ) -} - -async fn test_round_trip_ascii_password() { - let name = "test_round_trip_ascii_password".to_string(); - let entry = Entry::new(&name, &name).expect("Failed to create entry"); - let password = "test ascii password"; - entry.set_password(password).await.unwrap(); - let stored_password = entry.get_password().await.unwrap(); - assert_eq!(stored_password, password); - entry.delete_credential().await.unwrap(); - assert!(matches!(entry.get_password().await, Err(Error::NoEntry))) -} - -async fn test_round_trip_non_ascii_password() { - let name = "test_round_trip_non_ascii_password".to_string(); - let entry = Entry::new(&name, &name).expect("Failed to create entry"); - let password = "このきれいな花は桜です"; - entry.set_password(password).await.unwrap(); - let stored_password = entry.get_password().await.unwrap(); - assert_eq!(stored_password, password); - entry.delete_credential().await.unwrap(); - assert!(matches!(entry.get_password().await, Err(Error::NoEntry))) -} - -async fn test_update_password() { - let name = "test_update_password".to_string(); - let entry = Entry::new(&name, &name).expect("Failed to create entry"); - let password = "test ascii password"; - entry.set_password(password).await.unwrap(); - let stored_password = entry.get_password().await.unwrap(); - assert_eq!(stored_password, password); - let password = "このきれいな花は桜です"; - entry.set_password(password).await.unwrap(); - let stored_password = entry.get_password().await.unwrap(); - assert_eq!(stored_password, password); - entry.delete_credential().await.unwrap(); - assert!(matches!(entry.get_password().await, Err(Error::NoEntry))) -} diff --git a/crates/uv-keyring/src/credential.rs b/crates/uv-keyring/src/credential.rs index 965540d01..c208c536c 100644 --- a/crates/uv-keyring/src/credential.rs +++ b/crates/uv-keyring/src/credential.rs @@ -3,10 +3,10 @@ # Platform-independent secure storage model This module defines a plug and play model for platform-specific credential stores. -The model comprises two traits: [CredentialBuilderApi] for the underlying store -and [CredentialApi] for the entries in the store. These traits must be implemented -in a thread-safe way, a requirement captured in the [CredentialBuilder] and -[Credential] types that wrap them. +The model comprises two traits: [`CredentialBuilderApi`] for the underlying store +and [`CredentialApi`] for the entries in the store. These traits must be implemented +in a thread-safe way, a requirement captured in the [`CredentialBuilder`] and +[`Credential`] types that wrap them. */ use std::any::Any; use std::collections::HashMap; @@ -117,7 +117,7 @@ impl std::fmt::Debug for Credential { } /// A descriptor for the lifetime of stored credentials, returned from -/// a credential store's [persistence](CredentialBuilderApi::persistence) call. +/// a credential store's [`persistence`](CredentialBuilderApi::persistence) call. #[non_exhaustive] pub enum CredentialPersistence { /// Credentials vanish when the entry vanishes (stored in the entry) @@ -161,7 +161,7 @@ impl std::fmt::Debug for CredentialBuilder { } } -/// A thread-safe implementation of the [CredentialBuilder API](CredentialBuilderApi). +/// A thread-safe implementation of the [`CredentialBuilder` API](CredentialBuilderApi). pub type CredentialBuilder = dyn CredentialBuilderApi + Send + Sync; struct NopCredentialBuilder; diff --git a/crates/uv-keyring/src/error.rs b/crates/uv-keyring/src/error.rs index 9f7de2268..9f7e86c76 100644 --- a/crates/uv-keyring/src/error.rs +++ b/crates/uv-keyring/src/error.rs @@ -18,7 +18,7 @@ use crate::Credential; /// which may be platform-specific. /// /// This enum is non-exhaustive so that more values can be added to it -/// without a SemVer break. Clients should always have default handling +/// without a `SemVer` break. Clients should always have default handling /// for variants they don't understand. #[non_exhaustive] pub enum Error { @@ -64,27 +64,27 @@ pub type Result = std::result::Result; impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { match self { - Error::PlatformFailure(err) => write!(f, "Platform secure storage failure: {err}"), - Error::NoStorageAccess(err) => { + Self::PlatformFailure(err) => write!(f, "Platform secure storage failure: {err}"), + Self::NoStorageAccess(err) => { write!(f, "Couldn't access platform secure storage: {err}") } - Error::NoEntry => write!(f, "No matching entry found in secure storage"), - Error::BadEncoding(_) => write!(f, "Data is not UTF-8 encoded"), - Error::TooLong(name, len) => write!( + Self::NoEntry => write!(f, "No matching entry found in secure storage"), + Self::BadEncoding(_) => write!(f, "Data is not UTF-8 encoded"), + Self::TooLong(name, len) => write!( f, "Attribute '{name}' is longer than platform limit of {len} chars" ), - Error::Invalid(attr, reason) => { + Self::Invalid(attr, reason) => { write!(f, "Attribute {attr} is invalid: {reason}") } - Error::Ambiguous(items) => { + Self::Ambiguous(items) => { write!( f, "Entry is matched by {} credentials: {items:?}", items.len(), ) } - Error::NoDefaultCredentialBuilder => { + Self::NoDefaultCredentialBuilder => { write!( f, "No default credential builder is available; set one before creating entries" @@ -97,8 +97,8 @@ impl std::fmt::Display for Error { impl std::error::Error for Error { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { - Error::PlatformFailure(err) => Some(err.as_ref()), - Error::NoStorageAccess(err) => Some(err.as_ref()), + Self::PlatformFailure(err) => Some(err.as_ref()), + Self::NoStorageAccess(err) => Some(err.as_ref()), _ => None, } } diff --git a/crates/uv-keyring/src/lib.rs b/crates/uv-keyring/src/lib.rs index 07a012bcf..a6995657c 100644 --- a/crates/uv-keyring/src/lib.rs +++ b/crates/uv-keyring/src/lib.rs @@ -12,7 +12,7 @@ Linux, FreeBSD, OpenBSD, Windows, -macOS, and iOS. +and macOS. ## Design @@ -33,13 +33,13 @@ as well as which values are allowed for _target_ by that store. The abstract behavior of entries and credential stores are captured by two types (with associated traits): -- a _credential builder_, represented by the [CredentialBuilder] type - (and [CredentialBuilderApi](credential::CredentialBuilderApi) trait). Credential +- a _credential builder_, represented by the [`CredentialBuilder`] type + (and [`CredentialBuilderApi`](credential::CredentialBuilderApi) trait). Credential builders are given the identifying information (and target, if any) provided for an entry and map it to the identifying information for a platform-specific credential. -- a _credential_, represented by the [Credential] type - (and [CredentialApi](credential::CredentialApi) trait). The platform-specific credential +- a _credential_, represented by the [`Credential`] type + (and [`CredentialApi`](credential::CredentialApi) trait). The platform-specific credential identified by the builder for an entry is what provides the secure storage for that entry's password/secret. @@ -47,7 +47,7 @@ by two types (with associated traits): This crate runs on several different platforms, and on each one it provides (by default) an implementation of a default credential store used -on that platform (see [default_credential_builder]). +on that platform (see [`default_credential_builder`]). These implementations work by mapping the data used to identify an entry to data used to identify platform-specific storage objects. For example, on macOS, the service and user provided for an entry @@ -60,9 +60,9 @@ the one used by this crate to identify entries. These keystores expose their specific model in the concrete credential objects they use to implement the Credential trait. In order to allow clients to access this richer model, the Credential trait -has an [as_any](credential::CredentialApi::as_any) method that returns a +has an [`as_any`](credential::CredentialApi::as_any) method that returns a reference to the underlying -concrete object typed as [Any](std::any::Any), so that it can be downgraded to +concrete object typed as [`Any`](std::any::Any), so that it can be downgraded to its concrete type. ### Credential store features @@ -76,23 +76,15 @@ is built with a macOS target. The available credential store features, listed here, are all included in the default feature set: -- `apple-native`: Provides access to the Keychain credential store on macOS and iOS. +- `apple-native`: Provides access to the Keychain credential store on macOS. - `windows-native`: Provides access to the Windows Credential Store on Windows. -- `secret-service`: Provides access to the DBus-based - [Secret Service](https://specifications.freedesktop.org/secret-service/latest/) - storage on Linux, FreeBSD, and OpenBSD. This keystore provides - support for encrypting secrets when they are transferred across the bus, - specify the `encrypted` feature if you want to use this support. - By default, this keystore requires that the DBus library be - installed on the user's machine, - but you can avoid this requirement by specifying the `vendored` feature - (which will cause the build to include a static build of the dbus library). +- `secret-service`: Provides access to Secret Service. If you suppress the default feature set when building this crate, and you don't separately specify one of the included keystore features for your platform, -then no keystore will be built in, and calls to [Entry::new] and [Entry::new_with_target] +then no keystore will be built in, and calls to [`Entry::new`] and [`Entry::new_with_target`] will fail unless the client brings their own keystore (see next section). ## Client-provided Credential Stores @@ -102,13 +94,13 @@ are free to provide their own keystores and use those. There are two mechanisms provided for this: - Clients can give their desired credential builder to the crate - for use by the [Entry::new] and [Entry::new_with_target] calls. - This is done by making a call to [set_default_credential_builder]. + for use by the [`Entry::new`] and [`Entry::new_with_target`] calls. + This is done by making a call to [`set_default_credential_builder`]. The major advantage of this approach is that client code remains independent of the credential builder being used. - Clients can construct their concrete credentials directly and - then turn them into entries by using the [Entry::new_with_credential] + then turn them into entries by using the [`Entry::new_with_credential`] call. The major advantage of this approach is that credentials can be identified however clients want, rather than being restricted to the simple model used by this crate. @@ -119,7 +111,7 @@ In addition to the platform-specific credential stores, this crate always provides a mock credential store that clients can use to test their code in a platform independent way. The mock credential store allows for pre-setting errors as well as password values to -be returned from [Entry] method calls. If you want to use the mock +be returned from [`Entry`] method calls. If you want to use the mock credential store as your default in tests, make this call: ``` uv_keyring::set_default_credential_builder(uv_keyring::mock::default_credential_builder()) @@ -145,11 +137,11 @@ view the storage module documentation for your desired platform.) This module expects passwords to be UTF-8 encoded strings, so if a third party has stored an arbitrary byte string then retrieving that as a password will return a -[BadEncoding](Error::BadEncoding) error. +[`BadEncoding`](Error::BadEncoding) error. The returned error will have the raw bytes attached, so you can access them, but you can also just fetch -them directly using [get_secret](Entry::get_secret) rather than -[get_password](Entry::get_password). +them directly using [`get_secret`](Entry::get_secret) rather than +[`get_password`](Entry::get_password). While this crate's code is thread-safe, the underlying credential stores may not handle access from different threads reliably. @@ -161,7 +153,6 @@ Service, accesses from multiple threads (and even the same thread very quickly) are not recommended, as they may cause the RPC mechanism to fail. */ -use log::debug; use std::collections::HashMap; pub use credential::{Credential, CredentialBuilder}; @@ -212,7 +203,7 @@ static DEFAULT_BUILDER: std::sync::RwLock = /// This is really meant for use by clients who bring their own credential /// store and want to use it everywhere. If you are using multiple credential /// stores and want precise control over which credential is in which store, -/// then use [new_with_credential](Entry::new_with_credential). +/// then use [`new_with_credential`](Entry::new_with_credential). /// /// This will block waiting for all other threads currently creating entries /// to complete what they are doing. It's really meant to be called @@ -233,8 +224,6 @@ pub fn default_credential_builder() -> Box { return secret_service::default_credential_builder(); #[cfg(all(target_os = "macos", feature = "apple-native"))] return macos::default_credential_builder(); - #[cfg(all(target_os = "ios", feature = "apple-native"))] - return ios::default_credential_builder(); #[cfg(all(target_os = "windows", feature = "windows-native"))] return windows::default_credential_builder(); #[cfg(not(any( @@ -242,7 +231,6 @@ pub fn default_credential_builder() -> Box { all(target_os = "freebsd", feature = "secret-service"), all(target_os = "openbsd", feature = "secret-service"), all(target_os = "macos", feature = "apple-native"), - all(target_os = "ios", feature = "apple-native"), all(target_os = "windows", feature = "windows-native"), )))] credential::nop_credential_builder() @@ -271,86 +259,77 @@ impl Entry { /// /// # Errors /// - /// This function will return an [Error] if the `service` or `user` values are invalid. + /// This function will return an [`Error`] if the `service` or `user` values are invalid. /// The specific reasons for invalidity are platform-dependent, but include length constraints. /// /// # Panics /// - /// In the very unlikely event that the internal credential builder's `RwLock`` is poisoned, this function + /// In the very unlikely event that the internal credential builder's `RwLock` is poisoned, this function /// will panic. If you encounter this, and especially if you can reproduce it, please report a bug with the /// details (and preferably a backtrace) so the developers can investigate. - pub fn new(service: &str, user: &str) -> Result { - debug!("creating entry with service {service}, user {user}, and no target"); + pub fn new(service: &str, user: &str) -> Result { let entry = build_default_credential(None, service, user)?; - debug!("created entry {:?}", entry.inner); Ok(entry) } /// Create an entry for the given target, service, and user. /// /// The default credential builder is used. - pub fn new_with_target(target: &str, service: &str, user: &str) -> Result { - debug!("creating entry with service {service}, user {user}, and target {target}"); + pub fn new_with_target(target: &str, service: &str, user: &str) -> Result { let entry = build_default_credential(Some(target), service, user)?; - debug!("created entry {:?}", entry.inner); Ok(entry) } /// Create an entry from a credential that may be in any credential store. - pub fn new_with_credential(credential: Box) -> Entry { - debug!("create entry from {credential:?}"); - Entry { inner: credential } + pub fn new_with_credential(credential: Box) -> Self { + Self { inner: credential } } /// Set the password for this entry. /// - /// Can return an [Ambiguous](Error::Ambiguous) error + /// Can return an [`Ambiguous`](Error::Ambiguous) error /// if there is more than one platform credential /// that matches this entry. This can only happen /// on some platforms, and then only if a third-party /// application wrote the ambiguous credential. pub async fn set_password(&self, password: &str) -> Result<()> { - debug!("set password for entry {:?}", self.inner); self.inner.set_password(password).await } /// Set the secret for this entry. /// - /// Can return an [Ambiguous](Error::Ambiguous) error + /// Can return an [`Ambiguous`](Error::Ambiguous) error /// if there is more than one platform credential /// that matches this entry. This can only happen /// on some platforms, and then only if a third-party /// application wrote the ambiguous credential. pub async fn set_secret(&self, secret: &[u8]) -> Result<()> { - debug!("set secret for entry {:?}", self.inner); self.inner.set_secret(secret).await } /// Retrieve the password saved for this entry. /// - /// Returns a [NoEntry](Error::NoEntry) error if there isn't one. + /// Returns a [`NoEntry`](Error::NoEntry) error if there isn't one. /// - /// Can return an [Ambiguous](Error::Ambiguous) error + /// Can return an [`Ambiguous`](Error::Ambiguous) error /// if there is more than one platform credential /// that matches this entry. This can only happen /// on some platforms, and then only if a third-party /// application wrote the ambiguous credential. pub async fn get_password(&self) -> Result { - debug!("get password from entry {:?}", self.inner); self.inner.get_password().await } /// Retrieve the secret saved for this entry. /// - /// Returns a [NoEntry](Error::NoEntry) error if there isn't one. + /// Returns a [`NoEntry`](Error::NoEntry) error if there isn't one. /// - /// Can return an [Ambiguous](Error::Ambiguous) error + /// Can return an [`Ambiguous`](Error::Ambiguous) error /// if there is more than one platform credential /// that matches this entry. This can only happen /// on some platforms, and then only if a third-party /// application wrote the ambiguous credential. pub async fn get_secret(&self) -> Result> { - debug!("get secret from entry {:?}", self.inner); self.inner.get_secret().await } @@ -360,15 +339,14 @@ impl Entry { /// that can be set to string values. See the documentation for each credential store /// for a list of which attribute names are supported by that store. /// - /// Returns a [NoEntry](Error::NoEntry) error if there isn't a credential for this entry. + /// Returns a [`NoEntry`](Error::NoEntry) error if there isn't a credential for this entry. /// - /// Can return an [Ambiguous](Error::Ambiguous) error + /// Can return an [`Ambiguous`](Error::Ambiguous) error /// if there is more than one platform credential /// that matches this entry. This can only happen /// on some platforms, and then only if a third-party /// application wrote the ambiguous credential. pub async fn get_attributes(&self) -> Result> { - debug!("get attributes from entry {:?}", self.inner); self.inner.get_attributes().await } @@ -380,26 +358,22 @@ impl Entry { /// cross-platform use, each credential store ignores (without error) any specified attributes /// that aren't supported by that store. /// - /// Returns a [NoEntry](Error::NoEntry) error if there isn't a credential for this entry. + /// Returns a [`NoEntry`](Error::NoEntry) error if there isn't a credential for this entry. /// - /// Can return an [Ambiguous](Error::Ambiguous) error + /// Can return an [`Ambiguous`](Error::Ambiguous) error /// if there is more than one platform credential /// that matches this entry. This can only happen /// on some platforms, and then only if a third-party /// application wrote the ambiguous credential. pub async fn update_attributes(&self, attributes: &HashMap<&str, &str>) -> Result<()> { - debug!( - "update attributes for entry {:?} from map {attributes:?}", - self.inner - ); self.inner.update_attributes(attributes).await } /// Delete the underlying credential for this entry. /// - /// Returns a [NoEntry](Error::NoEntry) error if there isn't one. + /// Returns a [`NoEntry`](Error::NoEntry) error if there isn't one. /// - /// Can return an [Ambiguous](Error::Ambiguous) error + /// Can return an [`Ambiguous`](Error::Ambiguous) error /// if there is more than one platform credential /// that matches this entry. This can only happen /// on some platforms, and then only if a third-party @@ -409,7 +383,6 @@ impl Entry { /// structure, which is controlled by Rust. It only /// affects the underlying credential store. pub async fn delete_credential(&self) -> Result<()> { - debug!("delete entry {:?}", self.inner); self.inner.delete_credential().await } @@ -430,9 +403,6 @@ doc_comment::doctest!("../README.md", readme); /// There are no actual tests in this module. /// Instead, it contains generics that each keystore invokes in their tests, /// passing their store-specific parameters for the generic ones. -// -// Since iOS doesn't use any of these generics, we allow dead code. -#[allow(dead_code)] mod tests { use super::{Entry, Error, Result, credential::CredentialApi}; use std::collections::HashMap; @@ -451,25 +421,6 @@ mod tests { } } - /// Create a platform-specific credential given the constructor, service, user, and attributes - pub(crate) fn entry_from_constructor_and_attributes( - f: F, - service: &str, - user: &str, - attrs: &HashMap<&str, &str>, - ) -> Entry - where - F: FnOnce(Option<&str>, &str, &str, &HashMap<&str, &str>) -> Result, - T: 'static + CredentialApi + Send + Sync, - { - match f(None, service, user, attrs) { - Ok(credential) => Entry::new_with_credential(Box::new(credential)), - Err(err) => { - panic!("Couldn't create entry (service: {service}, user: {user}): {err:?}") - } - } - } - async fn test_round_trip_no_delete(case: &str, entry: &Entry, in_pass: &str) { entry .set_password(in_pass) @@ -482,7 +433,7 @@ mod tests { assert_eq!( in_pass, out_pass, "Passwords don't match for {case}: set='{in_pass}', get='{out_pass}'", - ) + ); } /// A basic round-trip unit test given an entry and a password. @@ -555,7 +506,7 @@ mod tests { assert!( matches!(entry.get_password().await, Err(Error::NoEntry)), "Missing entry has password" - ) + ); } pub(crate) async fn test_empty_password(f: F) diff --git a/crates/uv-keyring/src/macos.rs b/crates/uv-keyring/src/macos.rs index 45437d91f..4ca853b78 100644 --- a/crates/uv-keyring/src/macos.rs +++ b/crates/uv-keyring/src/macos.rs @@ -6,12 +6,8 @@ All credentials on macOS are stored in secure stores called _keychains_. The OS automatically creates three of them that live on filesystem, called _User_ (aka login), _Common_, and _System_. In addition, removable media can contain a keychain which can be registered under the name _Dynamic_. -Finally, on Apple Silicon devices, there is a more highly protected keychain -(called the _Data Protection_ or simply _Protected_ keychain). This is the same -keychain that is used by apps on iOS; so this module actually returns -iOS credentials for entries in the Data Protection keychain. -The target attribute of an [Entry](crate::Entry) determines (case-insensitive) +The target attribute of an [`Entry`](crate::Entry) determines (case-insensitive) which keychain that entry's credential is created in or searched for. If the entry has no target, or the specified target doesn't name (case-insensitive) one of the keychains listed above, the 'User' keychain is used. @@ -80,7 +76,7 @@ impl CredentialApi for MacCredential { /// Look up the password for this entry, if any. /// - /// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no + /// Returns a [`NoEntry`](ErrorCode::NoEntry) error if there is no /// credential in the store. async fn get_password(&self) -> Result { let (password_bytes, _) = @@ -91,7 +87,7 @@ impl CredentialApi for MacCredential { /// Look up the secret for this entry, if any. /// - /// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no + /// Returns a [`NoEntry`](ErrorCode::NoEntry) error if there is no /// credential in the store. async fn get_secret(&self) -> Result> { let (password_bytes, _) = @@ -102,7 +98,7 @@ impl CredentialApi for MacCredential { /// Delete the underlying generic credential for this entry, if any. /// - /// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no + /// Returns a [`NoEntry`](ErrorCode::NoEntry) error if there is no /// credential in the store. async fn delete_credential(&self) -> Result<()> { let (_, item) = @@ -113,7 +109,7 @@ impl CredentialApi for MacCredential { } /// Return the underlying concrete object with an `Any` type so that it can - /// be downgraded to a [MacCredential] for platform-specific processing. + /// be downgraded to a [`MacCredential`] for platform-specific processing. fn as_any(&self) -> &dyn std::any::Any { self } @@ -141,7 +137,7 @@ impl MacCredential { /// /// Creating a credential does not put anything into the keychain. /// The keychain entry will be created - /// when [set_password](MacCredential::set_password) is + /// when [`set_password`](MacCredential::set_password) is /// called. /// /// This will fail if the service or user strings are empty, @@ -178,7 +174,7 @@ impl MacCredential { } /// The builder for Mac keychain credentials -pub struct MacCredentialBuilder {} +pub struct MacCredentialBuilder; /// Returns an instance of the Mac credential builder. /// @@ -189,7 +185,7 @@ pub fn default_credential_builder() -> Box { } impl CredentialBuilderApi for MacCredentialBuilder { - /// Build a [MacCredential] for the given target, service, and user. + /// Build a [`MacCredential`] for the given target, service, and user. /// /// If a target is specified but not recognized as a keychain name, /// the User keychain is selected. @@ -207,7 +203,7 @@ impl CredentialBuilderApi for MacCredentialBuilder { } /// Return the underlying builder object with an `Any` type so that it can - /// be downgraded to a [MacCredentialBuilder] for platform-specific processing. + /// be downgraded to a [`MacCredentialBuilder`] for platform-specific processing. fn as_any(&self) -> &dyn std::any::Any { self } @@ -226,11 +222,11 @@ pub enum MacKeychainDomain { impl std::fmt::Display for MacKeychainDomain { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - MacKeychainDomain::User => "User".fmt(f), - MacKeychainDomain::System => "System".fmt(f), - MacKeychainDomain::Common => "Common".fmt(f), - MacKeychainDomain::Dynamic => "Dynamic".fmt(f), - MacKeychainDomain::Protected => "Protected".fmt(f), + Self::User => "User".fmt(f), + Self::System => "System".fmt(f), + Self::Common => "Common".fmt(f), + Self::Dynamic => "Dynamic".fmt(f), + Self::Protected => "Protected".fmt(f), } } } @@ -245,12 +241,12 @@ impl std::str::FromStr for MacKeychainDomain { /// or else we assume the login keychain is meant. fn from_str(s: &str) -> Result { match s.to_ascii_lowercase().as_str() { - "user" => Ok(MacKeychainDomain::User), - "system" => Ok(MacKeychainDomain::System), - "common" => Ok(MacKeychainDomain::Common), - "dynamic" => Ok(MacKeychainDomain::Dynamic), - "protected" => Ok(MacKeychainDomain::Protected), - "data protection" => Ok(MacKeychainDomain::Protected), + "user" => Ok(Self::User), + "system" => Ok(Self::System), + "common" => Ok(Self::Common), + "dynamic" => Ok(Self::Dynamic), + "protected" => Ok(Self::Protected), + "data protection" => Ok(Self::Protected), _ => Err(ErrorCode::Invalid( "target".to_string(), format!("'{s}' is not User, System, Common, Dynamic, or Protected"), @@ -301,7 +297,7 @@ mod tests { assert!(matches!( default_credential_builder().persistence(), CredentialPersistence::UntilDelete - )) + )); } fn entry_new(service: &str, user: &str) -> Entry { @@ -399,7 +395,7 @@ mod tests { assert!( matches!(mac_cred.domain, super::MacKeychainDomain::User), "wrong domain for unknown specifier" - ) + ); } } } diff --git a/crates/uv-keyring/src/mock.rs b/crates/uv-keyring/src/mock.rs index d9111ec2c..891cb0291 100644 --- a/crates/uv-keyring/src/mock.rs +++ b/crates/uv-keyring/src/mock.rs @@ -16,10 +16,10 @@ keyring::set_default_credential_builder(keyring::mock::default_credential_builde You can then create entries as you usually do, and call their usual methods to set, get, and delete passwords. There is no persistence other than in the entry itself, so getting a password before setting it will always result -in a [NoEntry](Error::NoEntry) error. +in a [`NoEntry`](Error::NoEntry) error. If you want a method call on an entry to fail in a specific way, you can -downcast the entry to a [MockCredential] and then call [set_error](MockCredential::set_error) +downcast the entry to a [`MockCredential`] and then call [`set_error`](MockCredential::set_error) with the appropriate error. The next entry method called on the credential will fail with the error you set. The error will then be cleared, so the next call on the mock will operate as usual. Here's a complete example: @@ -53,7 +53,7 @@ pub struct MockCredential { impl Default for MockCredential { fn default() -> Self { Self { - inner: Mutex::new(RefCell::new(Default::default())), + inner: Mutex::new(RefCell::new(MockData::default())), } } } @@ -187,8 +187,8 @@ impl MockCredential { /// /// Since mocks have no persistence between sessions, /// new mocks always have no password. - fn new_with_target(_target: Option<&str>, _service: &str, _user: &str) -> Result { - Ok(Default::default()) + fn new_with_target(_target: Option<&str>, _service: &str, _user: &str) -> Self { + Self::default() } /// Set an error to be returned from this mock credential. @@ -207,7 +207,7 @@ impl MockCredential { } /// The builder for mock credentials. -pub struct MockCredentialBuilder {} +pub struct MockCredentialBuilder; impl CredentialBuilderApi for MockCredentialBuilder { /// Build a mock credential for the given target, service, and user. @@ -215,7 +215,7 @@ impl CredentialBuilderApi for MockCredentialBuilder { /// Since mocks don't persist between sessions, all mocks /// start off without passwords. fn build(&self, target: Option<&str>, service: &str, user: &str) -> Result> { - let credential = MockCredential::new_with_target(target, service, user)?; + let credential = MockCredential::new_with_target(target, service, user); Ok(Box::new(credential)) } @@ -246,11 +246,11 @@ mod tests { assert!(matches!( default_credential_builder().persistence(), CredentialPersistence::EntryOnly - )) + )); } fn entry_new(service: &str, user: &str) -> Entry { - let credential = MockCredential::new_with_target(None, service, user).unwrap(); + let credential = MockCredential::new_with_target(None, service, user); Entry::new_with_credential(Box::new(credential)) } @@ -336,6 +336,6 @@ mod tests { assert!( matches!(entry.get_password().await, Err(Error::NoEntry)), "Able to read a deleted ascii password" - ) + ); } } diff --git a/crates/uv-keyring/src/secret_service.rs b/crates/uv-keyring/src/secret_service.rs index 7bbb3acb6..74ead8982 100644 --- a/crates/uv-keyring/src/secret_service.rs +++ b/crates/uv-keyring/src/secret_service.rs @@ -88,7 +88,7 @@ use crate::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,7 +104,7 @@ 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 + /// 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` @@ -115,7 +115,7 @@ 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 + /// 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` @@ -127,7 +127,7 @@ impl CredentialApi for SsCredential { .map_err(platform_failure)?; match self.matching_items(&ss).await { Ok(items) => { - for item in items.iter() { + for item in &items { set_item_secret(item, secret).await?; } return Ok(()); @@ -137,7 +137,7 @@ impl CredentialApi for SsCredential { } // 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,8 +162,8 @@ 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) @@ -183,9 +183,9 @@ impl CredentialApi for SsCredential { /// 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) @@ -236,8 +236,8 @@ impl CredentialApi for SsCredential { /// 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) @@ -260,7 +260,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 +273,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 { @@ -350,8 +350,8 @@ impl SsCredential { /// 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 @@ -359,7 +359,7 @@ impl SsCredential { match self.matching_items(&ss).await { Ok(items) => { let mut passwords = Vec::new(); - for item in items.iter() { + for item in &items { passwords.push(get_item_password(item).await?); } Ok(passwords) @@ -368,7 +368,7 @@ impl SsCredential { 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.iter() { + for item in &items { passwords.push(get_item_password(item).await?); } Ok(passwords) @@ -379,15 +379,15 @@ impl SsCredential { /// 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<()> { let ss = SecretService::connect(EncryptionType::Dh) .await .map_err(platform_failure)?; match self.matching_items(&ss).await { Ok(items) => { - for item in items.iter() { + for item in &items { delete_item(item).await?; } Ok(()) @@ -395,7 +395,7 @@ impl SsCredential { 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.iter() { + for item in &items { delete_item(item).await?; } Ok(()) @@ -428,8 +428,8 @@ impl SsCredential { /// 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 + /// 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?; @@ -443,9 +443,9 @@ impl SsCredential { 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)) + creds.push(Box::new(cred)); } - return Err(ErrorCode::Ambiguous(creds)); + Err(ErrorCode::Ambiguous(creds)) } } } @@ -474,7 +474,7 @@ impl SsCredential { /// 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 + /// 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() { @@ -496,9 +496,9 @@ impl SsCredential { /// 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 + /// 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 + /// 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>( &self, @@ -515,11 +515,11 @@ impl SsCredential { .search_items(attributes) .await .map_err(decode_error)?; - for item in search.iter() { + for item in &search { let cred = Self::new_from_item(item).await?; - creds.push(Box::new(cred)) + creds.push(Box::new(cred)); } - return Err(ErrorCode::Ambiguous(creds)); + Err(ErrorCode::Ambiguous(creds)) } } } @@ -535,7 +535,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 +550,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 +561,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 +569,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 } @@ -660,10 +660,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.iter() { + for (k, v) in &existing { updated.insert(k, v); } - for (k, v) in attributes.iter() { + for (k, v) in attributes { if k.eq(&"target") || k.eq(&"service") || k.eq(&"username") { continue; } @@ -735,7 +735,7 @@ mod tests { assert!(matches!( default_credential_builder().persistence(), CredentialPersistence::UntilDelete - )) + )); } fn entry_new(service: &str, user: &str) -> Entry { @@ -803,7 +803,7 @@ mod tests { actual.attributes.get(key).expect("Missing attribute"), value, "Attribute mismatch" - ) + ); } entry .delete_credential() diff --git a/crates/uv-keyring/src/windows.rs b/crates/uv-keyring/src/windows.rs index e374ed9b5..142bc86c4 100644 --- a/crates/uv-keyring/src/windows.rs +++ b/crates/uv-keyring/src/windows.rs @@ -21,10 +21,10 @@ So if you have a custom algorithm you want to use for computing the Windows targ you can specify the target name directly. (You still need to provide a service and username, because they are used in the credential's metadata.) -The [get_attributes](crate::Entry::get_attributes) +The [`get_attributes`](crate::Entry::get_attributes) call will return the values in the `username`, `comment`, and `target_alias` fields (using those strings as the attribute names), -and the [update_attributes](crate::Entry::update_attributes) +and the [`update_attributes`](crate::Entry::update_attributes) call allows setting those fields. ## Caveat @@ -36,6 +36,8 @@ shown that modifying the same entry in the same (almost simultaneous) order from different threads produces different results on different runs. */ +#![allow(unsafe_code)] + use crate::credential::{Credential, CredentialApi, CredentialBuilder, CredentialBuilderApi}; use crate::error::{Error as ErrorCode, Result}; use byteorder::{ByteOrder, LittleEndian}; @@ -71,6 +73,7 @@ pub struct WinCredential { // BOOL is i32 (false = 0, true = 1) // PCREDENTIALW = *mut CREDENTIALW +#[async_trait::async_trait] impl CredentialApi for WinCredential { /// Create and write a credential with password for this entry. /// @@ -86,7 +89,7 @@ impl CredentialApi for WinCredential { let mut blob_u16 = to_wstr_no_null(password); let mut blob = vec![0; blob_u16.len() * 2]; LittleEndian::write_u16_into(&blob_u16, &mut blob); - let result = self.set_secret(&blob); + let result = self.set_secret(&blob).await; // make sure that the copies of the secret are erased blob_u16.zeroize(); blob.zeroize(); @@ -105,7 +108,7 @@ impl CredentialApi for WinCredential { /// Look up the password for this entry, if any. /// - /// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no + /// Returns a [`NoEntry`](ErrorCode::NoEntry) error if there is no /// credential in the store. async fn get_password(&self) -> Result { self.extract_from_platform(extract_password) @@ -113,7 +116,7 @@ impl CredentialApi for WinCredential { /// Look up the secret for this entry, if any. /// - /// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no + /// Returns a [`NoEntry`](ErrorCode::NoEntry) error if there is no /// credential in the store. async fn get_secret(&self) -> Result> { self.extract_from_platform(extract_secret) @@ -121,7 +124,7 @@ impl CredentialApi for WinCredential { /// Get the attributes from the credential for this entry, if it exists. /// - /// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no + /// Returns a [`NoEntry`](ErrorCode::NoEntry) error if there is no /// credential in the store. async fn get_attributes(&self) -> Result> { let cred = self.extract_from_platform(Self::extract_credential)?; @@ -134,19 +137,19 @@ impl CredentialApi for WinCredential { /// Update the attributes on the credential for this entry, if it exists. /// - /// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no + /// Returns a [`NoEntry`](ErrorCode::NoEntry) error if there is no /// credential in the store. async fn update_attributes(&self, attributes: &HashMap<&str, &str>) -> Result<()> { let secret = self.extract_from_platform(extract_secret)?; let mut cred = self.extract_from_platform(Self::extract_credential)?; if let Some(comment) = attributes.get(&"comment") { - cred.comment = comment.to_string(); + cred.comment = (*comment).to_string(); } if let Some(target_alias) = attributes.get(&"target_alias") { - cred.target_alias = target_alias.to_string(); + cred.target_alias = (*target_alias).to_string(); } if let Some(username) = attributes.get(&"username") { - cred.username = username.to_string(); + cred.username = (*username).to_string(); } cred.validate_attributes(Some(&secret), None)?; cred.save_credential(&secret) @@ -154,7 +157,7 @@ impl CredentialApi for WinCredential { /// Delete the underlying generic credential for this entry, if any. /// - /// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no + /// Returns a [`NoEntry`](ErrorCode::NoEntry) error if there is no /// credential in the store. async fn delete_credential(&self) -> Result<()> { self.validate_attributes(None, None)?; @@ -167,12 +170,12 @@ impl CredentialApi for WinCredential { } /// Return the underlying concrete object with an `Any` type so that it can - /// be downgraded to a [WinCredential] for platform-specific processing. + /// be downgraded to a [`WinCredential`] for platform-specific processing. fn as_any(&self) -> &dyn std::any::Any { 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) } @@ -234,6 +237,7 @@ impl WinCredential { /// Write this credential into the underlying store as a Generic credential /// /// You must always have validated attributes before you call this! + #[allow(clippy::cast_possible_truncation)] fn save_credential(&self, secret: &[u8]) -> Result<()> { let mut username = to_wstr(&self.username); let mut target_name = to_wstr(&self.target_name); @@ -304,28 +308,26 @@ impl WinCredential { ) } }; - match result { - 0 => { - // `CredReadW` failed, so no allocation has been done, so no free needs to be done - Err(decode_error()) - } - _ => { - // `CredReadW` succeeded, so p_credential points at an allocated credential. - // To do anything with it, we need to cast it to the right type. That takes two steps: - // first we remove the "uninitialized" guard around it, then we reinterpret it as a - // pointer to the right structure type. - let p_credential = unsafe { p_credential.assume_init() }; - let w_credential: CREDENTIALW = unsafe { *p_credential }; - // Now we can apply the passed extractor function to the credential. - let result = f(&w_credential); - // Finally, we erase the secret and free the allocated credential. - erase_secret(&w_credential); - unsafe { CredFree(p_credential as *mut _) }; - result - } + if result == 0 { + // `CredReadW` failed, so no allocation has been done, so no free needs to be done + Err(decode_error()) + } else { + // `CredReadW` succeeded, so p_credential points at an allocated credential. + // To do anything with it, we need to cast it to the right type. That takes two steps: + // first we remove the "uninitialized" guard around it, then we reinterpret it as a + // pointer to the right structure type. + let p_credential = unsafe { p_credential.assume_init() }; + let w_credential: CREDENTIALW = unsafe { *p_credential }; + // Now we can apply the passed extractor function to the credential. + let result = f(&w_credential); + // Finally, we erase the secret and free the allocated credential. + erase_secret(&w_credential); + unsafe { CredFree(p_credential as *mut _) }; + result } } + #[allow(clippy::unnecessary_wraps)] fn extract_credential(w_credential: &CREDENTIALW) -> Result { Ok(Self { username: unsafe { from_wstr(w_credential.UserName) }, @@ -340,21 +342,11 @@ impl WinCredential { /// Creating a credential does not create a matching Generic credential /// in the Windows Credential Manager. /// If there isn't already one there, it will be created only - /// when [set_password](WinCredential::set_password) is + /// when [`set_password`](WinCredential::set_password) is /// called. - pub fn new_with_target( - target: Option<&str>, - service: &str, - user: &str, - ) -> Result { + pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result { const VERSION: &str = env!("CARGO_PKG_VERSION"); let credential = if let Some(target) = target { - // if target.is_empty() { - // return Err(ErrorCode::Invalid( - // "target".to_string(), - // "cannot be empty".to_string(), - // )); - // } Self { // On Windows, the target name is all that's used to // search for the credential, so we allow clients to @@ -386,7 +378,7 @@ impl WinCredential { } /// The builder for Windows Generic credentials. -pub struct WinCredentialBuilder {} +pub struct WinCredentialBuilder; /// Returns an instance of the Windows credential builder. /// @@ -397,7 +389,7 @@ pub fn default_credential_builder() -> Box { } impl CredentialBuilderApi for WinCredentialBuilder { - /// Build a [WinCredential] for the given target, service, and user. + /// Build a [`WinCredential`] for the given target, service, and user. fn build(&self, target: Option<&str>, service: &str, user: &str) -> Result> { Ok(Box::new(WinCredential::new_with_target( target, service, user, @@ -405,7 +397,7 @@ impl CredentialBuilderApi for WinCredentialBuilder { } /// Return the underlying builder object with an `Any` type so that it can - /// be downgraded to a [WinCredentialBuilder] for platform-specific processing. + /// be downgraded to a [`WinCredentialBuilder`] for platform-specific processing. fn as_any(&self) -> &dyn std::any::Any { self } @@ -435,6 +427,7 @@ fn extract_password(credential: &CREDENTIALW) -> Result { result } +#[allow(clippy::unnecessary_wraps)] fn extract_secret(credential: &CREDENTIALW) -> Result> { let blob_pointer: *const u8 = credential.CredentialBlob; let blob_len: usize = credential.CredentialBlobSize as usize; @@ -463,6 +456,7 @@ fn to_wstr_no_null(s: &str) -> Vec { s.encode_utf16().collect() } +#[allow(clippy::maybe_infinite_iter)] unsafe fn from_wstr(ws: *const u16) -> String { // null pointer case, return empty string if ws.is_null() { diff --git a/crates/uv-keyring/tests/basic.rs b/crates/uv-keyring/tests/basic.rs index 9f9b5cd42..ee7133983 100644 --- a/crates/uv-keyring/tests/basic.rs +++ b/crates/uv-keyring/tests/basic.rs @@ -12,7 +12,7 @@ async fn test_missing_entry() { assert!( matches!(entry.get_password().await, Err(Error::NoEntry)), "Missing entry has password" - ) + ); } #[tokio::test] @@ -42,7 +42,7 @@ async fn test_empty_password() { assert!( matches!(entry.get_password().await, Err(Error::NoEntry)), "Able to read a deleted password" - ) + ); } #[tokio::test] @@ -71,7 +71,7 @@ async fn test_round_trip_ascii_password() { assert!( matches!(entry.get_password().await, Err(Error::NoEntry)), "Able to read a deleted ascii password" - ) + ); } #[tokio::test] @@ -100,7 +100,7 @@ async fn test_round_trip_non_ascii_password() { assert!( matches!(entry.get_password().await, Err(Error::NoEntry)), "Able to read a deleted non-ascii password" - ) + ); } #[tokio::test] @@ -127,7 +127,7 @@ async fn test_round_trip_random_secret() { assert!( matches!(entry.get_password().await, Err(Error::NoEntry)), "Able to read a deleted random secret" - ) + ); } #[tokio::test] @@ -169,5 +169,5 @@ async fn test_update() { assert!( matches!(entry.get_password().await, Err(Error::NoEntry)), "Able to read a deleted updated password" - ) + ); } diff --git a/crates/uv-keyring/tests/threading.rs b/crates/uv-keyring/tests/threading.rs index 7a610bacf..e2389755d 100644 --- a/crates/uv-keyring/tests/threading.rs +++ b/crates/uv-keyring/tests/threading.rs @@ -1,3 +1,5 @@ +#![cfg(feature = "keyring-tests")] + use common::{generate_random_string, init_logger}; use uv_keyring::{Entry, Error}; @@ -9,50 +11,45 @@ async fn test_create_then_move() { let name = generate_random_string(); let entry = Entry::new(&name, &name).unwrap(); - let test = move || { - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .unwrap(); - runtime.block_on(async move { - let password = "test ascii password"; - entry - .set_password(password) - .await - .expect("Can't set initial ascii password"); - let stored_password = entry - .get_password() - .await - .expect("Can't get ascii password"); - assert_eq!( - stored_password, password, - "Retrieved and set initial ascii passwords don't match" - ); - let password = "このきれいな花は桜です"; - entry - .set_password(password) - .await - .expect("Can't set non-ascii password"); - let stored_password = entry - .get_password() - .await - .expect("Can't get non-ascii password"); - assert_eq!( - stored_password, password, - "Retrieved and set non-ascii passwords don't match" - ); - entry - .delete_credential() - .await - .expect("Can't delete non-ascii password"); - assert!( - matches!(entry.get_password().await, Err(Error::NoEntry)), - "Able to read a deleted non-ascii password" - ); - }); - }; - let handle = std::thread::spawn(test); - assert!(handle.join().is_ok(), "Couldn't execute on thread") + + let handle = tokio::spawn(async move { + let password = "test ascii password"; + entry + .set_password(password) + .await + .expect("Can't set initial ascii password"); + let stored_password = entry + .get_password() + .await + .expect("Can't get ascii password"); + assert_eq!( + stored_password, password, + "Retrieved and set initial ascii passwords don't match" + ); + let password = "このきれいな花は桜です"; + entry + .set_password(password) + .await + .expect("Can't set non-ascii password"); + let stored_password = entry + .get_password() + .await + .expect("Can't get non-ascii password"); + assert_eq!( + stored_password, password, + "Retrieved and set non-ascii passwords don't match" + ); + entry + .delete_credential() + .await + .expect("Can't delete non-ascii password"); + assert!( + matches!(entry.get_password().await, Err(Error::NoEntry)), + "Able to read a deleted non-ascii password" + ); + }); + + handle.await.expect("Task failed"); } #[tokio::test] @@ -63,38 +60,34 @@ async fn test_simultaneous_create_then_move() { for i in 0..10 { let name = format!("{}-{}", generate_random_string(), i); let entry = Entry::new(&name, &name).expect("Can't create entry"); - let test = move || { - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .unwrap(); - runtime.block_on(async move { - entry - .set_password(&name) - .await - .expect("Can't set ascii password"); - let stored_password = entry - .get_password() - .await - .expect("Can't get ascii password"); - assert_eq!( - stored_password, name, - "Retrieved and set ascii passwords don't match" - ); - entry - .delete_credential() - .await - .expect("Can't delete ascii password"); - assert!( - matches!(entry.get_password().await, Err(Error::NoEntry)), - "Able to read a deleted ascii password" - ); - }); - }; - handles.push(std::thread::spawn(test)) + + let handle = tokio::spawn(async move { + entry + .set_password(&name) + .await + .expect("Can't set ascii password"); + let stored_password = entry + .get_password() + .await + .expect("Can't get ascii password"); + assert_eq!( + stored_password, name, + "Retrieved and set ascii passwords don't match" + ); + entry + .delete_credential() + .await + .expect("Can't delete ascii password"); + assert!( + matches!(entry.get_password().await, Err(Error::NoEntry)), + "Able to read a deleted ascii password" + ); + }); + handles.push(handle); } + for handle in handles { - handle.join().expect("Couldn't execute on thread") + handle.await.expect("Task failed"); } } @@ -110,32 +103,27 @@ async fn test_create_set_then_move() { .set_password(password) .await .expect("Can't set ascii password"); - let test = move || { - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .unwrap(); - runtime.block_on(async move { - let stored_password = entry - .get_password() - .await - .expect("Can't get ascii password"); - assert_eq!( - stored_password, password, - "Retrieved and set ascii passwords don't match" - ); - entry - .delete_credential() - .await - .expect("Can't delete ascii password"); - assert!( - matches!(entry.get_password().await, Err(Error::NoEntry)), - "Able to read a deleted ascii password" - ); - }); - }; - let handle = std::thread::spawn(test); - assert!(handle.join().is_ok(), "Couldn't execute on thread") + + let handle = tokio::spawn(async move { + let stored_password = entry + .get_password() + .await + .expect("Can't get ascii password"); + assert_eq!( + stored_password, password, + "Retrieved and set ascii passwords don't match" + ); + entry + .delete_credential() + .await + .expect("Can't delete ascii password"); + assert!( + matches!(entry.get_password().await, Err(Error::NoEntry)), + "Able to read a deleted ascii password" + ); + }); + + handle.await.expect("Task failed"); } #[tokio::test] @@ -151,34 +139,30 @@ async fn test_simultaneous_create_set_then_move() { .set_password(&name) .await .expect("Can't set ascii password"); - let test = move || { - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .unwrap(); - runtime.block_on(async move { - let stored_password = entry - .get_password() - .await - .expect("Can't get ascii password"); - assert_eq!( - stored_password, name, - "Retrieved and set ascii passwords don't match" - ); - entry - .delete_credential() - .await - .expect("Can't delete ascii password"); - assert!( - matches!(entry.get_password().await, Err(Error::NoEntry)), - "Able to read a deleted ascii password" - ); - }); - }; - handles.push(std::thread::spawn(test)) + + let handle = tokio::spawn(async move { + let stored_password = entry + .get_password() + .await + .expect("Can't get ascii password"); + assert_eq!( + stored_password, name, + "Retrieved and set ascii passwords don't match" + ); + entry + .delete_credential() + .await + .expect("Can't delete ascii password"); + assert!( + matches!(entry.get_password().await, Err(Error::NoEntry)), + "Able to read a deleted ascii password" + ); + }); + handles.push(handle); } + for handle in handles { - handle.join().expect("Couldn't execute on thread") + handle.await.expect("Task failed"); } } @@ -189,39 +173,34 @@ async fn test_simultaneous_independent_create_set() { let mut handles = vec![]; for i in 0..10 { let name = format!("thread_entry{i}"); - let test = move || { - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .unwrap(); - runtime.block_on(async move { - let entry = Entry::new(&name, &name).expect("Can't create entry"); - entry - .set_password(&name) - .await - .expect("Can't set ascii password"); - let stored_password = entry - .get_password() - .await - .expect("Can't get ascii password"); - assert_eq!( - stored_password, name, - "Retrieved and set ascii passwords don't match" - ); - entry - .delete_credential() - .await - .expect("Can't delete ascii password"); - assert!( - matches!(entry.get_password().await, Err(Error::NoEntry)), - "Able to read a deleted ascii password" - ); - }); - }; - handles.push(std::thread::spawn(test)) + let handle = tokio::spawn(async move { + let entry = Entry::new(&name, &name).expect("Can't create entry"); + entry + .set_password(&name) + .await + .expect("Can't set ascii password"); + let stored_password = entry + .get_password() + .await + .expect("Can't get ascii password"); + assert_eq!( + stored_password, name, + "Retrieved and set ascii passwords don't match" + ); + entry + .delete_credential() + .await + .expect("Can't delete ascii password"); + assert!( + matches!(entry.get_password().await, Err(Error::NoEntry)), + "Able to read a deleted ascii password" + ); + }); + handles.push(handle); } + for handle in handles { - handle.join().expect("Couldn't execute on thread") + handle.await.expect("Task failed"); } } @@ -265,42 +244,37 @@ async fn test_simultaneous_multiple_create_delete_single_thread() { let mut handles = vec![]; for t in 0..10 { let name = generate_random_string(); - let test = move || { - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .unwrap(); - runtime.block_on(async move { - let name = format!("{name}-{t}"); - let entry = Entry::new(&name, &name).expect("Can't create entry"); - let repeats = 10; - for _i in 0..repeats { - entry - .set_password(&name) - .await - .expect("Can't set ascii password"); - let stored_password = entry - .get_password() - .await - .expect("Can't get ascii password"); - assert_eq!( - stored_password, name, - "Retrieved and set ascii passwords don't match" - ); - entry - .delete_credential() - .await - .expect("Can't delete ascii password"); - assert!( - matches!(entry.get_password().await, Err(Error::NoEntry)), - "Able to read a deleted ascii password" - ); - } - }); - }; - handles.push(std::thread::spawn(test)) + let handle = tokio::spawn(async move { + let name = format!("{name}-{t}"); + let entry = Entry::new(&name, &name).expect("Can't create entry"); + let repeats = 10; + for _i in 0..repeats { + entry + .set_password(&name) + .await + .expect("Can't set ascii password"); + let stored_password = entry + .get_password() + .await + .expect("Can't get ascii password"); + assert_eq!( + stored_password, name, + "Retrieved and set ascii passwords don't match" + ); + entry + .delete_credential() + .await + .expect("Can't delete ascii password"); + assert!( + matches!(entry.get_password().await, Err(Error::NoEntry)), + "Able to read a deleted ascii password" + ); + } + }); + handles.push(handle); } + for handle in handles { - handle.join().expect("Couldn't execute on thread") + handle.await.expect("Task failed"); } }