diff --git a/cmd/passless/src/authenticator.rs b/cmd/passless/src/authenticator.rs index 55b31d4..9b03048 100644 --- a/cmd/passless/src/authenticator.rs +++ b/cmd/passless/src/authenticator.rs @@ -58,11 +58,15 @@ impl AuthenticatorCallbacks for PasslessCallbacks { self.security_config.user_verification_authentication }; - // If backend handles verification (e.g., GPG) and not registration, skip notification - if self.storage.lock().unwrap().disable_user_verification() - && !is_registration - && !should_verify - { + let storage = match self.storage.lock() { + Ok(s) => s, + Err(_) => { + error!("Failed to acquire storage lock during user verification request"); + return Err(soft_fido2::Error::Other); + } + }; + + if storage.disable_user_verification() && !is_registration && !should_verify { debug!("User verification handled by backend (e.g., GPG): {}", info); return Ok(UpResult::Accepted); } @@ -113,7 +117,15 @@ impl AuthenticatorCallbacks for PasslessCallbacks { fn write_credential(&self, credential: &CredentialRef) -> Result<()> { info!("Storing credential for RP: {}", credential.rp_id); debug!("Credential ID: {}", bytes_to_hex(credential.id)); - let mut storage = self.storage.lock().unwrap(); + + let mut storage = match self.storage.lock() { + Ok(s) => s, + Err(_) => { + error!("Failed to acquire storage lock while writing credential"); + return Err(soft_fido2::Error::Other); + } + }; + storage.write(*credential)?; info!( "Credential persisted successfully for RP: {}", @@ -124,7 +136,14 @@ impl AuthenticatorCallbacks for PasslessCallbacks { fn read_credential(&self, cred_id: &[u8]) -> Result> { debug!("Reading credential: id={}", bytes_to_hex(cred_id)); - let mut storage = self.storage.lock().unwrap(); + + let mut storage = match self.storage.lock() { + Ok(s) => s, + Err(_) => { + error!("Failed to acquire storage lock while reading credential"); + return Err(soft_fido2::Error::Other); + } + }; match storage.read(cred_id) { Ok(cred) => { @@ -140,7 +159,15 @@ impl AuthenticatorCallbacks for PasslessCallbacks { fn delete_credential(&self, cred_id: &[u8]) -> Result<()> { info!("Removing credential ID: {}", bytes_to_hex(cred_id)); - let mut storage = self.storage.lock().unwrap(); + + let mut storage = match self.storage.lock() { + Ok(s) => s, + Err(_) => { + error!("Failed to acquire storage lock while deleting credential"); + return Err(soft_fido2::Error::Other); + } + }; + storage.delete(cred_id)?; debug!("Credential removed"); Ok(()) @@ -148,7 +175,14 @@ impl AuthenticatorCallbacks for PasslessCallbacks { fn list_credentials(&self, rp_id: &str, _user_id: Option<&[u8]>) -> Result> { info!("Listing credentials for RP: {}", rp_id); - let mut storage = self.storage.lock().unwrap(); + + let mut storage = match self.storage.lock() { + Ok(s) => s, + Err(_) => { + error!("Failed to acquire storage lock while listing credentials"); + return Err(soft_fido2::Error::Other); + } + }; let filter = CredentialFilter::ByRp(rp_id.to_string()); @@ -185,7 +219,14 @@ impl AuthenticatorCallbacks for PasslessCallbacks { fn enumerate_rps(&self) -> Result, usize)>> { debug!("Enumerating relying parties"); - let mut storage = self.storage.lock().unwrap(); + + let mut storage = match self.storage.lock() { + Ok(s) => s, + Err(_) => { + error!("Failed to acquire storage lock while enumerating RPs"); + return Err(soft_fido2::Error::Other); + } + }; // Get all credentials let filter = CredentialFilter::None; @@ -221,7 +262,15 @@ impl AuthenticatorCallbacks for PasslessCallbacks { fn credential_count(&self) -> Result { debug!("Counting total credentials"); - let storage = self.storage.lock().unwrap(); + + let storage = match self.storage.lock() { + Ok(s) => s, + Err(_) => { + error!("Failed to acquire storage lock while counting credentials"); + return Err(soft_fido2::Error::Other); + } + }; + let count = storage.count_credentials(); debug!("Total credentials: {}", count); Ok(count) @@ -339,8 +388,13 @@ mod tests { #[test] fn test_service_creation() { let temp_dir = std::env::temp_dir().join("test_passless"); - std::fs::create_dir_all(&temp_dir).unwrap(); - let storage = LocalStorageAdapter::new(temp_dir.clone()).unwrap(); + if let Err(e) = std::fs::create_dir_all(&temp_dir) { + panic!("Failed to create temp directory: {}", e); + } + let storage = match LocalStorageAdapter::new(temp_dir.clone()) { + Ok(s) => s, + Err(e) => panic!("Failed to create local storage: {}", e), + }; let security_config = SecurityConfig { check_mlock: false, @@ -352,7 +406,7 @@ mod tests { }; let service = AuthenticatorService::new(storage, security_config); - assert!(service.is_ok()); + assert!(service.is_ok(), "Service creation should succeed"); // Cleanup let _ = std::fs::remove_dir_all(temp_dir); diff --git a/cmd/passless/src/commands/client.rs b/cmd/passless/src/commands/client.rs index 2982de0..922357f 100644 --- a/cmd/passless/src/commands/client.rs +++ b/cmd/passless/src/commands/client.rs @@ -218,7 +218,9 @@ fn open_device_by_selector(list: &TransportList, selector: &str) -> Result { - let (_device_info, transport) = matches.into_iter().next().unwrap(); + let (_device_info, transport) = matches.into_iter().next().ok_or_else(|| { + passless_core::Error::Other("No device found after filtering".to_string()) + })?; Ok(transport) } _ => { diff --git a/cmd/passless/src/commands/custom.rs b/cmd/passless/src/commands/custom.rs index 2d7f134..0716b2e 100644 --- a/cmd/passless/src/commands/custom.rs +++ b/cmd/passless/src/commands/custom.rs @@ -152,11 +152,18 @@ mod tests { use passless_core::config::SecurityConfig; let temp_dir = std::env::temp_dir().join("test_passless_custom"); - std::fs::create_dir_all(&temp_dir).unwrap(); - let storage = LocalStorageAdapter::new(temp_dir.clone()).unwrap(); + if let Err(e) = std::fs::create_dir_all(&temp_dir) { + panic!("Failed to create temp directory: {}", e); + } + let storage = match LocalStorageAdapter::new(temp_dir.clone()) { + Ok(s) => s, + Err(e) => panic!("Failed to create local storage: {}", e), + }; + + let service = AuthenticatorService::new(storage, SecurityConfig::default()); + assert!(service.is_ok(), "Service creation should succeed"); - let mut service = AuthenticatorService::new(storage, SecurityConfig::default()).unwrap(); - register_yubikey_credential_mgmt(&mut service); + register_yubikey_credential_mgmt(&mut service.unwrap()); // Test that custom command was registered // (Further testing would require CTAP protocol simulation) diff --git a/cmd/passless/src/notification.rs b/cmd/passless/src/notification.rs index ce1cb80..08869c5 100644 --- a/cmd/passless/src/notification.rs +++ b/cmd/passless/src/notification.rs @@ -98,14 +98,16 @@ pub fn show_verification_notification( // Wait for user action handle.wait_for_action(|action| { debug!("User action received: {}", action); - let mut result = action_result_clone.lock().unwrap(); + let mut result = action_result_clone + .lock() + .expect("Failed to lock action result"); *result = Some(action.to_string()); }); // Process the action taken let action = action_result .lock() - .unwrap() + .expect("Failed to lock action result") .clone() .unwrap_or_else(|| "__closed".to_string()); @@ -174,14 +176,16 @@ pub fn show_yes_no_notification(title: &str, question: &str) -> Result Ok(Self::Gpgme), "gnupg-bin" | "gnupg_bin" | "gnupg" => Ok(Self::GnupgBin), - _ => Err(Error::Config(format!("Invalid GPG backend: {}", s))), + _ => Err(Error::Config(format!( + "Invalid GPG backend: '{}'. Must be 'gpgme' or 'gnupg-bin'", + s + ))), + } + } + + #[allow(dead_code)] + pub fn validate(&self) -> Result<()> { + match self { + Self::Gpgme => { + debug!("Validating GPGME backend configuration"); + Ok(()) + } + Self::GnupgBin => { + debug!("Validating GnuPG binary backend configuration"); + use std::process::Command; + let result = Command::new("gpg").arg("--version").output(); + + match result { + Ok(output) if output.status.success() => { + debug!( + "GPG binary is available: {:?}", + String::from_utf8_lossy(&output.stdout) + ); + Ok(()) + } + _ => { + warn!("GPG binary not found or not executable"); + Ok(()) // Don't fail, just warn + } + } + } } } }