Skip to content

Commit

Permalink
fix(secrets): Reduce keychain unlock prompts on MacOS
Browse files Browse the repository at this point in the history
Signed-off-by: Timothy Johnson <[email protected]>
  • Loading branch information
t1m0thyj committed Dec 21, 2024
1 parent aa2d6e6 commit 977580a
Show file tree
Hide file tree
Showing 5 changed files with 368 additions and 21 deletions.
4 changes: 4 additions & 0 deletions packages/secrets/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

All notable changes to the Zowe Secrets SDK package will be documented in this file.

## Recent Changes

- BugFix: Reduced number of keychain unlock prompts on MacOS for simultaneous access to secrets by the same process.

## `8.1.2`

- BugFix: Updated dependencies for technical currency. [#2289](https://github.com/zowe/zowe-cli/pull/2289)
Expand Down
173 changes: 164 additions & 9 deletions packages/secrets/core/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion packages/secrets/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ version = "0.48.0"
[target.'cfg(target_os = "macos")'.dependencies]
core-foundation = "0.9.3"
core-foundation-sys = "0.8.4"
named-lock = "0.4.1"

[target.'cfg(any(target_os = "freebsd", target_os = "linux"))'.dependencies]
glib = "0.18.2"
glib-sys = "0.18.1"
gio = "0.18.2"
libsecret = "0.4.0"
libsecret-sys = "0.4.0"
libsecret-sys = "0.4.0"
26 changes: 26 additions & 0 deletions packages/secrets/core/src/os/mac/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use error::Error;
use crate::os::mac::error::ERR_SEC_ITEM_NOT_FOUND;
use crate::os::mac::keychain_search::{KeychainSearch, SearchResult};
use keychain::SecKeychain;
use named_lock::NamedLock;

impl From<Error> for KeyringError {
fn from(error: Error) -> Self {
Expand All @@ -22,6 +23,26 @@ impl From<Error> for KeyringError {
}
}

impl From<named_lock::Error> for KeyringError {
fn from(error: named_lock::Error) -> Self {
KeyringError::Library {
name: "named-lock".to_owned(),
details: error.to_string(),
}
}
}

fn keyring_mutex() -> Result<NamedLock, KeyringError> {
// MacOS shows keychain prompt after secret has been modified by another process. We use cross-process mutex to
// block keychain access if there are multiple concurrent keychain operations invoked by the same process. This
// prevents multiple instances of the same app (e.g. VS Code) from triggering several keychain prompts at once.
let exe_path = std::env::current_exe()
.unwrap()
.to_string_lossy()
.replace(std::path::MAIN_SEPARATOR, "_");
NamedLock::create(format!("keyring_{}", exe_path).as_str()).map_err(|e| KeyringError::from(e))
}

///
/// Attempts to set a password for a given service and account.
///
Expand All @@ -38,6 +59,7 @@ pub fn set_password(
password: &String,
) -> Result<bool, KeyringError> {
let keychain = SecKeychain::default().unwrap();
let _lock = keyring_mutex()?.lock().unwrap();
match keychain.set_password(service.as_str(), account.as_str(), password.as_bytes()) {
Ok(()) => Ok(true),
Err(err) => Err(KeyringError::from(err)),
Expand All @@ -56,6 +78,7 @@ pub fn set_password(
///
pub fn get_password(service: &String, account: &String) -> Result<Option<String>, KeyringError> {
let keychain = SecKeychain::default().unwrap();
let _lock = keyring_mutex()?.lock().unwrap();
match keychain.find_password(service.as_str(), account.as_str()) {
Ok((pw, _)) => Ok(Some(String::from_utf8(pw.to_owned())?)),
Err(err) if err.code() == ERR_SEC_ITEM_NOT_FOUND => Ok(None),
Expand Down Expand Up @@ -83,6 +106,7 @@ pub fn find_password(service: &String) -> Result<Option<String>, KeyringError> {
}

let keychain = SecKeychain::default().unwrap();
let _lock = keyring_mutex()?.lock().unwrap();
match keychain.find_password(cred_attrs[0], cred_attrs[1]) {
Ok((pw, _)) => {
let pw_str = String::from_utf8(pw.to_owned())?;
Expand All @@ -104,6 +128,7 @@ pub fn find_password(service: &String) -> Result<Option<String>, KeyringError> {
///
pub fn delete_password(service: &String, account: &String) -> Result<bool, KeyringError> {
let keychain = SecKeychain::default().unwrap();
let _lock = keyring_mutex()?.lock().unwrap();
match keychain.find_password(service.as_str(), account.as_str()) {
Ok((_, item)) => {
item.delete()?;
Expand All @@ -128,6 +153,7 @@ pub fn find_credentials(
service: &String,
credentials: &mut Vec<(String, String)>,
) -> Result<bool, KeyringError> {
let _lock = keyring_mutex()?.lock().unwrap();
match KeychainSearch::new()
.label(service.as_str())
.with_attrs()
Expand Down
Loading

0 comments on commit 977580a

Please sign in to comment.