Skip to content

Commit

Permalink
replace tracing by log
Browse files Browse the repository at this point in the history
Also used env_logger for tests.
  • Loading branch information
soywod committed Oct 14, 2024
1 parent e0c336a commit 303424b
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 118 deletions.
8 changes: 6 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ jobs:
run: cargo clippy -- -D warnings

- name: Build and Test
run: cargo test --features=apple-native,windows-native --verbose
env:
RUST_LOG: debug
run: cargo test --features=apple-native,windows-native --verbose -- --nocapture

- name: Build the CLI release
run: cargo build --release --features=apple-native,windows-native --example keyring-cli
Expand Down Expand Up @@ -85,8 +87,10 @@ jobs:
run: gnome-keyring-daemon --components=secrets --daemonize --unlock <<< 'foobar'

- name: Run tests
env:
RUST_LOG: debug
# run tests single-threaded to avoid dbus race conditions
run: cargo test --features=${{ matrix.features }} -- --test-threads=1
run: cargo test --features=${{ matrix.features }} -- --test-threads=1 --nocapture

ios_native:
runs-on: macos-latest
Expand Down
7 changes: 4 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ async-io = ["zbus?/async-io"]
vendored = ["dbus-secret-service?/vendored", "openssl?/vendored"]

[dependencies]
log = "0.4.22"
openssl = { version = "0.10.55", optional = true }
tracing = { version = "0.1.40", optional = true }

[target.'cfg(any(target_os = "macos", target_os = "ios"))'.dependencies] # see issue #190
security-framework = { version = "3", optional = true }
Expand Down Expand Up @@ -64,10 +64,11 @@ path = "examples/cli.rs"
[dev-dependencies]
base64 = "0.22"
clap = { version = "4", features = ["derive", "wrap_help"] }
doc-comment = "0.3"
env_logger = "0.11.5"
rand = "0.8"
rpassword = "7"
rprompt = "2"
rand = "0.8"
doc-comment = "0.3"
whoami = "1"

[package.metadata.docs.rs]
Expand Down
12 changes: 0 additions & 12 deletions src/ios.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ impl CredentialApi for IosCredential {
/// Since there is only one credential with a given _account_ and _user_
/// in any given keychain, there is no chance of ambiguity.
fn set_secret(&self, secret: &[u8]) -> Result<()> {
#[cfg(feature = "tracing")]
tracing::debug!("set ios secret");

set_generic_password(&self.service, &self.account, secret).map_err(decode_error)?;
Ok(())
}
Expand All @@ -78,9 +75,6 @@ impl CredentialApi for IosCredential {
/// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no
/// credential in the store.
fn get_secret(&self) -> Result<Vec<u8>> {
#[cfg(feature = "tracing")]
tracing::debug!("get ios secret");

get_generic_password(&self.service, &self.account).map_err(decode_error)
}

Expand All @@ -89,9 +83,6 @@ impl CredentialApi for IosCredential {
/// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no
/// credential in the store.
fn delete_credential(&self) -> Result<()> {
#[cfg(feature = "tracing")]
tracing::debug!("delete ios credential");

delete_generic_password(&self.service, &self.account).map_err(decode_error)?;
Ok(())
}
Expand Down Expand Up @@ -132,9 +123,6 @@ impl IosCredential {
/// because empty attribute values act as wildcards in the
/// Keychain Services API.
pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result<Self> {
#[cfg(feature = "tracing")]
tracing::debug!(target?, service, user, "create ios credential");

if service.is_empty() {
return Err(ErrorCode::Invalid(
"service".to_string(),
Expand Down
12 changes: 0 additions & 12 deletions src/keyutils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,6 @@ impl CredentialApi for KeyutilsCredential {
}

fn set_secret(&self, secret: &[u8]) -> Result<()> {
#[cfg(feature = "tracing")]
tracing::debug!("set keyutils secret");

if secret.is_empty() {
return Err(ErrorCode::Invalid(
"secret".to_string(),
Expand Down Expand Up @@ -181,9 +178,6 @@ impl CredentialApi for KeyutilsCredential {
///
/// This requires a call to `Key::read`.
fn get_secret(&self) -> Result<Vec<u8>> {
#[cfg(feature = "tracing")]
tracing::debug!("get keyutils secret");

// Verify that the key exists and is valid
let key = self
.session
Expand Down Expand Up @@ -219,9 +213,6 @@ impl CredentialApi for KeyutilsCredential {
/// by get_password if it's called within milliseconds
/// in *the same process* that deleted the key.
fn delete_credential(&self) -> Result<()> {
#[cfg(feature = "tracing")]
tracing::debug!("delete keyutils credential");

// Verify that the key exists and is valid
let key = self
.session
Expand Down Expand Up @@ -264,9 +255,6 @@ impl KeyutilsCredential {
/// If none is provided, then we concatenate the user and service in the string
/// `keyring-rs:user@service`.
pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result<Self> {
#[cfg(feature = "tracing")]
tracing::debug!(?target, service, user, "create keyutils credential");

// Obtain the session keyring
let session =
KeyRing::from_special_id(KeyRingIdentifier::Session, false).map_err(decode_error)?;
Expand Down
21 changes: 18 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ are not recommended, as they may cause the RPC mechanism to fail.

pub use credential::{Credential, CredentialBuilder};
pub use error::{Error, Result};
use log::debug;
use std::collections::HashMap;

pub mod mock;
Expand Down Expand Up @@ -328,18 +329,25 @@ impl Entry {
/// 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<Entry> {
build_default_credential(None, service, user)
debug!("creating entry with service {service} and user {user}");
let entry = build_default_credential(None, service, user)?;
debug!("created entry with {:?}", 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<Entry> {
build_default_credential(Some(target), service, user)
debug!("creating entry with service {service}, user {user} and target {target:?}");
let entry = build_default_credential(Some(target), service, user)?;
debug!("created entry with {:?}", entry.inner);
Ok(entry)
}

/// Create an entry that uses the given platform credential for storage.
pub fn new_with_credential(credential: Box<Credential>) -> Entry {
debug!("create entry from {credential:?}");
Entry { inner: credential }
}

Expand All @@ -351,6 +359,7 @@ impl Entry {
/// on some platforms, and then only if a third-party
/// application wrote the ambiguous credential.
pub fn set_password(&self, password: &str) -> Result<()> {
debug!("set entry password using {:?}", self.inner);
self.inner.set_password(password)
}

Expand All @@ -362,6 +371,7 @@ impl Entry {
/// on some platforms, and then only if a third-party
/// application wrote the ambiguous credential.
pub fn set_secret(&self, secret: &[u8]) -> Result<()> {
debug!("set entry secret using {:?}", self.inner);
self.inner.set_secret(secret)
}

Expand All @@ -375,6 +385,7 @@ impl Entry {
/// on some platforms, and then only if a third-party
/// application wrote the ambiguous credential.
pub fn get_password(&self) -> Result<String> {
debug!("get entry password using {:?}", self.inner);
self.inner.get_password()
}

Expand All @@ -388,6 +399,7 @@ impl Entry {
/// on some platforms, and then only if a third-party
/// application wrote the ambiguous credential.
pub fn get_secret(&self) -> Result<Vec<u8>> {
debug!("get entry secret using {:?}", self.inner);
self.inner.get_secret()
}

Expand All @@ -405,6 +417,7 @@ impl Entry {
/// on some platforms, and then only if a third-party
/// application wrote the ambiguous credential.
pub fn get_attributes(&self) -> Result<HashMap<String, String>> {
debug!("get attributes from {:?}", self.inner);
self.inner.get_attributes()
}

Expand All @@ -424,6 +437,7 @@ impl Entry {
/// on some platforms, and then only if a third-party
/// application wrote the ambiguous credential.
pub fn update_attributes(&self, attributes: &HashMap<&str, &str>) -> Result<()> {
debug!("update attributes {attributes:?} from {:?}", self.inner);
self.inner.update_attributes(attributes)
}

Expand All @@ -441,6 +455,7 @@ impl Entry {
/// structure, which is controlled by Rust. It only
/// affects the underlying credential store.
pub fn delete_credential(&self) -> Result<()> {
debug!("delete {:?}", self.inner);
self.inner.delete_credential()
}

Expand Down Expand Up @@ -555,7 +570,7 @@ mod tests {
pub fn generate_random_string_of_len(len: usize) -> String {
// from the Rust Cookbook:
// https://rust-lang-nursery.github.io/rust-cookbook/algorithms/randomness.html
use rand::{distributions::Alphanumeric, thread_rng, Rng};
use rand::{distributions::Alphanumeric, thread_rng};
thread_rng()
.sample_iter(&Alphanumeric)
.take(len)
Expand Down
18 changes: 0 additions & 18 deletions src/macos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ impl CredentialApi for MacCredential {
/// Since there is only one credential with a given _account_ and _user_
/// in any given keychain, there is no chance of ambiguity.
fn set_password(&self, password: &str) -> Result<()> {
#[cfg(feature = "tracing")]
tracing::debug!("set macos password");

get_keychain(self)?
.set_generic_password(&self.service, &self.account, password.as_bytes())
.map_err(decode_error)?;
Expand All @@ -69,9 +66,6 @@ impl CredentialApi for MacCredential {
/// Since there is only one credential with a given _account_ and _user_
/// in any given keychain, there is no chance of ambiguity.
fn set_secret(&self, secret: &[u8]) -> Result<()> {
#[cfg(feature = "tracing")]
tracing::debug!("set macos secret");

get_keychain(self)?
.set_generic_password(&self.service, &self.account, secret)
.map_err(decode_error)?;
Expand All @@ -83,9 +77,6 @@ impl CredentialApi for MacCredential {
/// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no
/// credential in the store.
fn get_password(&self) -> Result<String> {
#[cfg(feature = "tracing")]
tracing::debug!("get macos password");

let (password_bytes, _) =
find_generic_password(Some(&[get_keychain(self)?]), &self.service, &self.account)
.map_err(decode_error)?;
Expand All @@ -97,9 +88,6 @@ impl CredentialApi for MacCredential {
/// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no
/// credential in the store.
fn get_secret(&self) -> Result<Vec<u8>> {
#[cfg(feature = "tracing")]
tracing::debug!("get macos secret");

let (password_bytes, _) =
find_generic_password(Some(&[get_keychain(self)?]), &self.service, &self.account)
.map_err(decode_error)?;
Expand All @@ -111,9 +99,6 @@ impl CredentialApi for MacCredential {
/// Returns a [NoEntry](ErrorCode::NoEntry) error if there is no
/// credential in the store.
fn delete_credential(&self) -> Result<()> {
#[cfg(feature = "tracing")]
tracing::debug!("delete macos credential");

let (_, item) =
find_generic_password(Some(&[get_keychain(self)?]), &self.service, &self.account)
.map_err(decode_error)?;
Expand Down Expand Up @@ -159,9 +144,6 @@ impl MacCredential {
/// because empty attribute values act as wildcards in the
/// Keychain Services API.
pub fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result<Self> {
#[cfg(feature = "tracing")]
tracing::debug!(?target, service, user, "create macos credential");

if service.is_empty() {
return Err(ErrorCode::Invalid(
"service".to_string(),
Expand Down
18 changes: 0 additions & 18 deletions src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@ impl CredentialApi for MockCredential {
/// and the password will _not_ be set. The error will
/// be cleared, so calling again will set the password.
fn set_password(&self, password: &str) -> Result<()> {
#[cfg(feature = "tracing")]
tracing::debug!("set mock password");

let mut inner = self.inner.lock().expect("Can't access mock data for set");
let data = inner.get_mut();
let err = data.error.take();
Expand All @@ -100,9 +97,6 @@ impl CredentialApi for MockCredential {
/// and the password will _not_ be set. The error will
/// be cleared, so calling again will set the password.
fn set_secret(&self, secret: &[u8]) -> Result<()> {
#[cfg(feature = "tracing")]
tracing::debug!("set mock secret");

let mut inner = self.inner.lock().expect("Can't access mock data for set");
let data = inner.get_mut();
let err = data.error.take();
Expand All @@ -120,9 +114,6 @@ impl CredentialApi for MockCredential {
/// If there is an error set in the mock, it will
/// be returned instead of a password.
fn get_password(&self) -> Result<String> {
#[cfg(feature = "tracing")]
tracing::debug!("get mock password");

let mut inner = self.inner.lock().expect("Can't access mock data for get");
let data = inner.get_mut();
let err = data.error.take();
Expand All @@ -140,9 +131,6 @@ impl CredentialApi for MockCredential {
/// If there is an error set in the mock, it will
/// be returned instead of a password.
fn get_secret(&self) -> Result<Vec<u8>> {
#[cfg(feature = "tracing")]
tracing::debug!("get mock secret");

let mut inner = self.inner.lock().expect("Can't access mock data for get");
let data = inner.get_mut();
let err = data.error.take();
Expand All @@ -163,9 +151,6 @@ impl CredentialApi for MockCredential {
/// If there is no password, a [NoEntry](Error::NoEntry) error
/// will be returned.
fn delete_credential(&self) -> Result<()> {
#[cfg(feature = "tracing")]
tracing::debug!("delete mock credential");

let mut inner = self
.inner
.lock()
Expand Down Expand Up @@ -204,9 +189,6 @@ impl MockCredential {
/// new mocks always have no password.
#[cfg_attr(not(feature = "tracing"), allow(unused_variables))]
fn new_with_target(target: Option<&str>, service: &str, user: &str) -> Result<Self> {
#[cfg(feature = "tracing")]
tracing::debug!(?target, service, user, "create mock credential");

Ok(Default::default())
}

Expand Down
Loading

0 comments on commit 303424b

Please sign in to comment.