Skip to content

Commit

Permalink
Merge pull request #2394 from zowe/fix/macos-keychain-prompts
Browse files Browse the repository at this point in the history
fix(secrets): Reduce keychain unlock prompts on MacOS
  • Loading branch information
t1m0thyj authored Jan 7, 2025
2 parents bf77e64 + 9040008 commit 5f86044
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 24 deletions.
29 changes: 15 additions & 14 deletions .github/workflows/secrets-sdk.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,27 +38,27 @@ jobs:
npm run build -- --target i686-pc-windows-msvc
npm run test
target: i686-pc-windows-msvc
- host: ubuntu-22.04
- host: ubuntu-latest
target: x86_64-unknown-linux-gnu
use-cross: true
build: |
set -e
CARGO=cross npm run build -- --target x86_64-unknown-linux-gnu
- host: ubuntu-22.04
- host: ubuntu-latest
target: i686-unknown-linux-gnu
use-cross: true
build: |
set -e
source scripts/configure-cross.sh i686-unknown-linux-gnu
CARGO=cross npm run build -- --target i686-unknown-linux-gnu
- host: ubuntu-22.04
- host: ubuntu-latest
target: armv7-unknown-linux-gnueabihf
use-cross: true
build: |
set -e
source scripts/configure-cross.sh armv7-unknown-linux-gnueabihf
CARGO=cross npm run build -- --target armv7-unknown-linux-gnueabihf
- host: ubuntu-22.04
- host: ubuntu-latest
target: x86_64-unknown-linux-musl
use-cross: true
build: |
Expand All @@ -67,14 +67,14 @@ jobs:
- host: macos-latest
target: aarch64-apple-darwin
build: npm run build -- --target aarch64-apple-darwin
- host: ubuntu-22.04
- host: ubuntu-latest
target: aarch64-unknown-linux-gnu
use-cross: true
build: |
set -e
source scripts/configure-cross.sh aarch64-unknown-linux-gnu
CARGO=cross npm run build -- --target aarch64-unknown-linux-gnu
- host: ubuntu-22.04
- host: ubuntu-latest
target: aarch64-unknown-linux-musl
use-cross: true
build: |
Expand Down Expand Up @@ -208,22 +208,23 @@ jobs:
- host: macos-latest
target: x86_64-apple-darwin
architecture: x64
- host: ubuntu-22.04
- host: ubuntu-latest
target: x86_64-unknown-linux-gnu
- host: ubuntu-22.04
- host: ubuntu-latest
target: x86_64-unknown-linux-musl
- host: ubuntu-22.04
- host: ubuntu-latest
target: aarch64-unknown-linux-gnu
platform: linux/arm64
- host: ubuntu-22.04
- host: ubuntu-latest
target: aarch64-unknown-linux-musl
platform: linux/arm64
- host: ubuntu-22.04
- host: ubuntu-latest
target: armv7-unknown-linux-gnueabihf
platform: linux/arm/v7
node:
- "18"
- "20"
- "22"
runs-on: ${{ matrix.settings.host }}
steps:
- uses: actions/checkout@v4
Expand All @@ -250,10 +251,10 @@ jobs:
if: ${{ matrix.settings.platform }}
- name: Test bindings
run: npm run test
if: ${{ matrix.settings.host != 'ubuntu-22.04' }}
if: ${{ matrix.settings.host != 'ubuntu-latest' }}
- name: Setup and run tests
uses: addnab/docker-run-action@v3
if: ${{ matrix.settings.host == 'ubuntu-22.04' && !endsWith(matrix.settings.target, 'musl') }}
if: ${{ matrix.settings.host == 'ubuntu-latest' && !endsWith(matrix.settings.target, 'musl') }}
with:
image: ${{ format('node:{0}-slim', matrix.node) }}
options: "-v ${{ github.workspace }}:/build -w /build --cap-add=IPC_LOCK ${{ matrix.settings.platform && format('--platform={0}', matrix.settings.platform) }}"
Expand All @@ -263,7 +264,7 @@ jobs:
cd packages/secrets && dbus-run-session -- bash scripts/linux-test.sh
- name: Setup and run tests (MUSL)
uses: addnab/docker-run-action@v3
if: ${{ matrix.settings.host == 'ubuntu-22.04' && endsWith(matrix.settings.target, 'musl') }}
if: ${{ matrix.settings.host == 'ubuntu-latest' && endsWith(matrix.settings.target, 'musl') }}
with:
image: ${{ format('node:{0}-alpine', matrix.node) }}
options: "-v ${{ github.workspace }}:/build -w /build --cap-add=IPC_LOCK ${{ matrix.settings.platform && format('--platform={0}', matrix.settings.platform) }}"
Expand Down
6 changes: 5 additions & 1 deletion 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 multiple instances of the same application. [#2394](https://github.com/zowe/zowe-cli/pull/2394)

## `8.1.2`

- BugFix: Updated dependencies for technical currency. [#2289](https://github.com/zowe/zowe-cli/pull/2289)
Expand Down Expand Up @@ -52,4 +56,4 @@ All notable changes to the Zowe Secrets SDK package will be documented in this f
## `7.18.0`

- Initial release.
- `keyring` module added for interacting with OS-specific keyring/credential vaults. See [src/keyring](src/keyring/README.md) for information on this native module and how it can be used.
- `keyring` module added for interacting with OS-specific keyring/credential vaults. See [src/keyring](src/keyring/README.md) for information on this native module and how it can be used.
12 changes: 11 additions & 1 deletion packages/secrets/core/Cargo.lock

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

5 changes: 3 additions & 2 deletions 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"
fmutex = "0.1.0"

[target.'cfg(any(target_os = "freebsd", target_os = "linux"))'.dependencies]
gio = "0.18.2"
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"
1 change: 0 additions & 1 deletion packages/secrets/core/src/os/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ pub enum KeyringError {
#[error("[keyring] {name:?} library returned an error:\n\n{details:?}")]
Library { name: String, details: String },

#[cfg(not(target_os = "macos"))]
#[error("[keyring] An OS error has occurred:\n\n{0}")]
Os(String),

Expand Down
27 changes: 27 additions & 0 deletions packages/secrets/core/src/os/mac/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use error::Error;

use crate::os::mac::error::ERR_SEC_ITEM_NOT_FOUND;
use crate::os::mac::keychain_search::{KeychainSearch, SearchResult};
use fmutex::Guard;
use keychain::SecKeychain;

impl From<Error> for KeyringError {
Expand All @@ -22,6 +23,27 @@ impl From<Error> for KeyringError {
}
}

impl From<std::io::Error> for KeyringError {
fn from(error: std::io::Error) -> Self {
KeyringError::Os(error.to_string())
}
}

fn keyring_mutex() -> Result<Guard, 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, "_");
let lock_path = std::env::temp_dir()
.join(format!("zowe_{}_{}.lock", env!("CARGO_PKG_NAME"), exe_path));
std::fs::OpenOptions::new().create(true).write(true).open(&lock_path)
.and_then(|_| fmutex::lock(lock_path))
.map_err(KeyringError::from)
}

///
/// Attempts to set a password for a given service and account.
///
Expand All @@ -38,6 +60,7 @@ pub fn set_password(
password: &String,
) -> Result<bool, KeyringError> {
let keychain = SecKeychain::default().unwrap();
let _lock = keyring_mutex().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 +79,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().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 +107,7 @@ pub fn find_password(service: &String) -> Result<Option<String>, KeyringError> {
}

let keychain = SecKeychain::default().unwrap();
let _lock = keyring_mutex().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 +129,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().unwrap();
match keychain.find_password(service.as_str(), account.as_str()) {
Ok((_, item)) => {
item.delete()?;
Expand All @@ -128,6 +154,7 @@ pub fn find_credentials(
service: &String,
credentials: &mut Vec<(String, String)>,
) -> Result<bool, KeyringError> {
let _lock = keyring_mutex().unwrap();
match KeychainSearch::new()
.label(service.as_str())
.with_attrs()
Expand Down
3 changes: 2 additions & 1 deletion packages/secrets/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
"ava": "^6.0.0"
},
"ava": {
"timeout": "3m"
"timeout": "3m",
"workerThreads": false
},
"engines": {
"node": ">=14"
Expand Down
12 changes: 11 additions & 1 deletion packages/secrets/src/keyring/Cargo.lock

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

6 changes: 3 additions & 3 deletions packages/secrets/src/keyring/Cross.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ passthrough = [
image = "ghcr.io/cross-rs/aarch64-unknown-linux-gnu:main"

[target.aarch64-unknown-linux-musl]
image = "rust:alpine"
image = "rust:alpine3.20"
pre-build = [
"wget -qO- https://musl.cc/aarch64-linux-musl-cross.tgz | tar -xzC / && export PATH=\"/aarch64-linux-musl-cross/bin:$PATH\"",
"apk add --no-cache musl-dev pkgconfig",
"apk add -p /aarch64-linux-musl-cross --initdb --arch aarch64 --allow-untrusted -X \"https://dl-cdn.alpinelinux.org/alpine/latest-stable/main/\" --no-cache --no-scripts libsecret-dev",
"apk add -p /aarch64-linux-musl-cross --initdb --arch aarch64 --allow-untrusted -X $(head -n 1 /etc/apk/repositories) --no-cache --no-scripts libsecret-dev",
"rustup target add aarch64-unknown-linux-musl"
]

Expand All @@ -33,5 +33,5 @@ image = "ghcr.io/cross-rs/i686-unknown-linux-gnu:main"
image = "ghcr.io/cross-rs/x86_64-unknown-linux-gnu:main"

[target.x86_64-unknown-linux-musl]
image = "rust:alpine"
image = "rust:alpine3.20"
pre-build = ["apk add libsecret-dev musl-dev"]

0 comments on commit 5f86044

Please sign in to comment.