Error handling improvements

.gitignore

@@ -1,4 +1,10 @@
+# don't check in Cargo.lock because this is a library project
+# see https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html
 Cargo.lock
+# don't check in .cargo directory because not every developer
+# uses it; the typical use is to enable cross-compiling.
+/.cargo/
+# don't check in the binary and testing artifacts
 target
 fuzz
 libtest.rmeta

Cargo.toml

@@ -9,9 +9,8 @@ repository = "https://github.com/hwchen/keyring-rs.git"
 version = "0.10.4"
 edition = "2018"
 
-[features]
-default = []
-macos-specify-keychain = []
+[dependencies]
+bytes = "1.1.0"
 
 [target.'cfg(target_os = "macos")'.dependencies]
 security-framework = "2.4.2"
@@ -28,6 +27,7 @@ clap = "2.33"
 rpassword = "5.0"
 rand = "0.8.4"
 serial_test = "0.5.1"
+doc-comment = "0.3.3"
 
 [target.'cfg(target_os = "macos")'.dev-dependencies]
 tempfile = "3.1.0"

README.md

@@ -15,122 +15,51 @@ __Currently supports Linux, macOS, and Windows.__ Please file issues if you have
 
 To use this library in your project add the following to your `Cargo.toml` file:
 
-```
+```toml
 [dependencies]
-keyring = "0.10.1"
+keyring = "0.10"
 ```
 
 This will give you access to the `keyring` crate in your code. Now you can use
 the `new` function to get an instance of the `Keyring` struct. The `new`
 function expects a `service` name and an `username` with which it accesses
 the password.
 
-You can get a password from the OS keyring with the `get_password` function.
+Passwords can be added to the keyring using the `set_password` function.  Then can then be read back using the `get_password` function, and deleted using the `delete_password` method.
 
-```rust,no_run
+```rust
 extern crate keyring;
 
 use std::error::Error;
 
 fn main() -> Result<(), Box<dyn Error>> {
-  let service = "my_application_name";
-  let username = "username";
-
-  let keyring = keyring::Keyring::new(&service, &username);
-
-  let password = keyring.get_password()?;
-  println!("The password is '{}'", password);
-
-  Ok(())
-}
-```
-
-Passwords can also be added to the keyring using the `set_password` function.
-
-```rust,no_run
-extern crate keyring;
+    let service = "my_application";
+    let username = "my_name";
+    let keyring = keyring::Keyring::new(&service, &username);
 
-use std::error::Error;
+    let password = "topS3cr3tP4$$w0rd";
+    keyring.set_password(&password)?;
 
-fn main() -> Result<(), Box<dyn Error>> {
-  let service = "my_application_name";
-  let username = "username";
+    let password = keyring.get_password()?;
+    println!("My password is '{}'", password);
 
-  let keyring = keyring::Keyring::new(&service, &username);
+    keyring.delete_password()?;
+    println!("My password has been deleted");
 
-  let password = "topS3cr3tP4$$w0rd";
-  keyring.set_password(&password)?;
-
-  let password = keyring.get_password()?;
-  println!("The password is '{}'", password);
-
-  Ok(())
-}
-```
-
-And they can be deleted with the `delete_password` function.
-
-```rust,no_run
-extern crate keyring;
-
-use std::error::Error;
-
-fn main() -> Result<(), Box<dyn Error>> {
-  let service = "my_application_name";
-  let username = "username";
-
-  let keyring = keyring::Keyring::new(&service, &username);
-
-  keyring.delete_password()?;
-
-  println!("The password has been deleted");
-
-  Ok(())
-}
-```
-
-On macOS, keychain object from specific path can be opened using `Keyring::use_keychain` which gives the flexibility to open non-default keychains. Note that this is currently feature-gated, and is considered unstable, and is subject to change without a semver major version change.
-
-In Cargo.toml, you need to turn the feature on:
-```toml
-keyring = { version = "0.10.0", features = ["macos-specify-keychain"] }
-```
-
-```rust,no_run
-extern crate keyring;
-
-use std::error::Error;
-
-fn main() -> Result<(), Box<dyn Error>> {
-  let service = "my_application_name";
-  let username = "username";
-
-  let keyring = keyring::Keyring::use_keychain(Path::new("/Library/Keychains/System.keychain"), &service, &username);
-
-  let password = "topS3cr3tP4$$w0rd";
-  keyring.set_password(&password)?;
-
-  let password = keyring.get_password()?;
-  println!("The password is '{}'", password);
-
-  Ok(())
+    Ok(())
 }
 ```
 
 ## Errors
 
-The `get_password`, `set_password` and `delete_password` functions return a
-`Result` which, if the operation was unsuccessful, can yield a `KeyringError`.
-
-The `KeyringError` struct implements the `error::Error` and `fmt::Display`
-traits, so it can be queried for a cause and an description using methods of
-the same name.
+The `get_password`, `set_password` and `delete_password` functions return a `Result` which, if the operation was unsuccessful, can yield a `keyring::Error` with a platform-independent code that describes the error and a platform-specific error that can be used to get more details.
 
 ## Caveats
 
+* This module manipulates passwords as UTF-8 encoded strings, so if a 3rd party has stored a non-Unicode password then retrieving that password will return an error.  The error in that case will have the raw bytes attached, so you can access them.
+
 ### Linux
 
-* The application name is hardcoded to be `rust-keyring`.
 * If you are running on a headless linux box, you will need to unlock the Gnome login keyring before you can use it.  The following `bash` function may be very helpful.
 ```shell
 function unlock-keyring ()
@@ -143,11 +72,11 @@ function unlock-keyring ()
 
 ### Windows
 
-* The credential name is currently hardcoded to be `username.service`, due to a [reported issue](https://github.com/jaraco/keyring/issues/47).  This breaks compatibility with 3rd-party applications, and is being fixed.
+* The default Windows approach to storing credentials doesn't allow storing passwords for different users against the same service.  You can override this approach, see the `IdentityMapper` type for details.
 
 ### MacOS
 
-* Accessing the keychain from multiple threads simultaneously is generally a bad idea, and can cause deadlocks.
+* Accessing the same keychain entry from multiple threads simultaneously is generally a bad idea, and can cause deadlocks.
 
 ## Dev Notes
 
@@ -178,6 +107,7 @@ Thanks to the following for helping make this library better, whether through co
 - @steveatinfincia
 - @bhkaminski
 - @MaikKlein
+- @brotskydotcom
 
 ### Contribution
 

examples/example.rs

@@ -7,13 +7,19 @@ fn main() -> Result<()> {
     let service = "example-service";
     let password = "example-password";
     let keyring = Keyring::new(service, username);
-    keyring.set_password(password)?;
-    let stored_password = keyring.get_password()?;
-    assert_eq!(
-        password, stored_password,
-        "Stored and retrieved passwords don't match"
-    );
-    keyring.delete_password()?;
+    if let Err(err) = keyring.set_password(password) {
+        panic!("Could not set password: {}", err)
+    }
+    match keyring.get_password() {
+        Ok(stored_password) => assert_eq!(
+            password, stored_password,
+            "Stored and retrieved passwords don't match"
+        ),
+        Err(err) => panic!("Could not get password: {}", err),
+    }
+    if let Err(err) = keyring.delete_password() {
+        panic!("Could not delete password: {}", err);
+    }
     assert!(
         keyring.get_password().is_err(),
         "No error retrieving password after deletion"

src/attrs.rs

@@ -0,0 +1,126 @@
+/*
+Every platform's secure storage system keeps a set of attributes with
+each stored item.  Which attributes are allowed can vary, as can which
+of the attributes are required and which are used to identify the item.
+
+The attribute model supported by this crate is that each item has only
+two attributes: username and service, and they are used together to
+uniquely identify the item.
+
+The mismatch between this crate's attribute model and the underlying
+platform's attribute model can lead to incompatibility with 3rd-party
+applications whose attribute model, while consistent with the underlying
+platform model, may be more or less fine-grained than this crate's model.
+
+For example:
+
+* On Windows, generic credential are identified by an arbitrary string,
+and that string may not be constructed by a third party application
+the same way this crate constructs it from username and service.
+* On Linux, additional attributes can be used by 3rd parties to produce
+credentials identified my more than just the two attributes this crate
+uses by default.
+
+Thus, to provide interoperability with 3rd party credential clients,
+we provide a way for clients of this crate to override this crate's
+default algorithm for how the username and service are combined so as to
+produce the platform-specific attributes that identify each item.
+ */
+
+use std::collections::HashMap;
+
+#[derive(Debug)]
+pub enum Platform {
+    Linux,
+    Windows,
+    MacOs,
+}
+
+#[derive(Debug, Clone)]
+pub struct LinuxCredential {
+    pub attributes: HashMap<String, String>,
+    pub label: String,
+}
+
+impl LinuxCredential {
+    pub fn attributes(&self) -> HashMap<&str, &str> {
+        self.attributes
+            .iter()
+            .map(|(k, v)| (k.as_str(), v.as_str()))
+            .collect()
+    }
+
+    pub fn label(&self) -> &str {
+        self.label.as_str()
+    }
+}
+
+#[derive(Debug, Clone)]
+pub struct WinCredential {
+    pub username: String,
+    pub target_name: String,
+    pub target_alias: String,
+    pub comment: String,
+}
+
+#[derive(Debug, Clone)]
+pub struct MacCredential {
+    pub service: String,
+    pub account: String,
+}
+
+#[derive(Debug, Clone)]
+pub enum PlatformCredential {
+    Linux(LinuxCredential),
+    Win(WinCredential),
+    Mac(MacCredential),
+}
+
+impl PlatformCredential {
+    pub fn matches_platform(&self, os: &Platform) -> bool {
+        match self {
+            PlatformCredential::Linux(_) => matches!(os, Platform::Linux),
+            PlatformCredential::Mac(_) => matches!(os, Platform::MacOs),
+            PlatformCredential::Win(_) => matches!(os, Platform::Windows),
+        }
+    }
+}
+
+// TODO: Make this a Fn trait so we can accept closures
+pub type CredentialMapper = fn(&Platform, &str, &str) -> PlatformCredential;
+
+pub fn default_credential_mapper(
+    platform: Platform,
+    service: &str,
+    username: &str,
+) -> PlatformCredential {
+    match platform {
+        Platform::Linux => PlatformCredential::Linux(LinuxCredential {
+            attributes: HashMap::from([
+                ("service".to_string(), service.to_string()),
+                ("username".to_string(), username.to_string()),
+                ("application".to_string(), "rust-keyring".to_string()),
+            ]),
+            label: format!(
+                "keyring-rs credential for service '{}', user '{}'",
+                service, username
+            ),
+        }),
+        Platform::Windows => PlatformCredential::Win(WinCredential {
+            // Note: default concatenation of user and service name is
+            // needed because windows identity is on target_name only
+            // See issue here: https://github.com/jaraco/keyring/issues/47
+            username: username.to_string(),
+            target_name: format!("{}.{}", username, service),
+            target_alias: String::new(),
+            comment: format!(
+                "keyring-rs credential for service '{}', user '{}'",
+                service, username
+            ),
+        }),
+        Platform::MacOs => PlatformCredential::Mac(MacCredential {
+            service: service.to_string(),
+            account: username.to_string(),
+        }),
+    }
+}