From 2717f681637626b95ae2a927a313fdb57324613e Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Tue, 13 Jan 2026 08:31:27 -0600 Subject: [PATCH] Add more detail to tests for readability --- .../autofill_provider/src/assertion.rs | 12 +-- .../autofill_provider/src/lib.rs | 92 +++++++++++++------ .../autofill_provider/src/lock_status.rs | 4 +- .../autofill_provider/src/registration.rs | 8 +- .../src/window_handle_query.rs | 4 +- 5 files changed, 76 insertions(+), 44 deletions(-) diff --git a/apps/desktop/desktop_native/autofill_provider/src/assertion.rs b/apps/desktop/desktop_native/autofill_provider/src/assertion.rs index 64b5c2baac5..3b4f0b5fc53 100644 --- a/apps/desktop/desktop_native/autofill_provider/src/assertion.rs +++ b/apps/desktop/desktop_native/autofill_provider/src/assertion.rs @@ -6,10 +6,10 @@ use serde::{Deserialize, Serialize}; use crate::TimedCallback; use crate::{BitwardenError, Callback, Position, UserVerification}; +/// Request to assert a credential. #[cfg_attr(target_os = "macos", derive(uniffi::Record))] #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -/// Request to assert a credential. pub struct PasskeyAssertionRequest { /// Relying Party ID for the request. pub rp_id: String, @@ -71,17 +71,17 @@ pub struct PasskeyAssertionWithoutUserInterfaceRequest { /// The allowed credential ID for the request. pub credential_id: Vec, - #[cfg(target_os = "macos")] /// The user name for the credential that was previously given to the OS. + #[cfg(target_os = "macos")] pub user_name: String, - #[cfg(target_os = "macos")] /// The user ID for the credential that was previously given to the OS. + #[cfg(target_os = "macos")] pub user_handle: Vec, - #[cfg(target_os = "macos")] /// The app-specific local identifier for the credential, in our case, the /// cipher ID. + #[cfg(target_os = "macos")] pub record_identifier: Option, /// SHA-256 hash of the `clientDataJSON` for the assertion request. @@ -102,7 +102,6 @@ pub struct PasskeyAssertionWithoutUserInterfaceRequest { /// On Windows, this must be logical pixels, not physical pixels. pub window_xy: Position, - #[cfg(not(target_os = "macos"))] /// Byte string representing the native OS window handle for the WebAuthn client. /// # Operating System Differences /// @@ -111,6 +110,7 @@ pub struct PasskeyAssertionWithoutUserInterfaceRequest { /// /// ## Windows /// On Windows, this is a HWND. + #[cfg(not(target_os = "macos"))] pub client_window_handle: Vec, // #[cfg(not(target_os = "macos"))] @@ -150,8 +150,8 @@ pub struct PasskeyAssertionResponse { pub credential_id: Vec, } -#[cfg_attr(target_os = "macos", uniffi::export(with_foreign))] /// Callback to process a response to passkey assertion request. +#[cfg_attr(target_os = "macos", uniffi::export(with_foreign))] pub trait PreparePasskeyAssertionCallback: Send + Sync { /// Function to call if a successful response is returned. fn on_complete(&self, credential: PasskeyAssertionResponse); diff --git a/apps/desktop/desktop_native/autofill_provider/src/lib.rs b/apps/desktop/desktop_native/autofill_provider/src/lib.rs index 36b4c7d2493..1da95323d4c 100644 --- a/apps/desktop/desktop_native/autofill_provider/src/lib.rs +++ b/apps/desktop/desktop_native/autofill_provider/src/lib.rs @@ -50,20 +50,20 @@ uniffi::setup_scaffolding!(); #[cfg(target_os = "macos")] static INIT: Once = Once::new(); +/// User verification preference for WebAuthn requests. #[cfg_attr(target_os = "macos", derive(uniffi::Enum))] #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -/// User verification preference for WebAuthn requests. pub enum UserVerification { Preferred, Required, Discouraged, } +/// Coordinates representing a point on the screen. #[cfg_attr(target_os = "macos", derive(uniffi::Record))] #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -/// Coordinates representing a point on the screen. pub struct Position { pub x: i32, pub y: i32, @@ -97,16 +97,15 @@ trait Callback: Send + Sync { fn error(&self, error: BitwardenError); } -#[cfg_attr(target_os = "macos", derive(uniffi::Enum))] -#[derive(Debug)] /// Store the connection status between the credential provider extension /// and the desktop application's IPC server. +#[cfg_attr(target_os = "macos", derive(uniffi::Enum))] +#[derive(Debug)] pub enum ConnectionStatus { Connected, Disconnected, } -#[cfg_attr(target_os = "macos", derive(uniffi::Object))] /// A client to send and receive messages to the autofill service on the desktop /// client. /// @@ -155,6 +154,7 @@ pub enum ConnectionStatus { /// // use client here /// } /// ``` +#[cfg_attr(target_os = "macos", derive(uniffi::Object))] pub struct AutofillProviderClient { to_server_send: tokio::sync::mpsc::Sender, @@ -167,10 +167,10 @@ pub struct AutofillProviderClient { connection_status: Arc, } -#[derive(Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] /// Store native desktop status information to use for IPC communication /// between the application and the credential provider. +#[derive(Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] pub struct NativeStatus { key: String, value: String, @@ -288,11 +288,11 @@ impl AutofillProviderClient { #[cfg_attr(target_os = "macos", uniffi::export)] impl AutofillProviderClient { - #[cfg_attr(target_os = "macos", uniffi::constructor)] /// Asynchronously initiates a connection to the autofill service on the desktop client. /// /// See documentation at the top-level of [this struct][AutofillProviderClient] for usage /// information. + #[cfg_attr(target_os = "macos", uniffi::constructor)] pub fn connect() -> Self { tracing::trace!("Autofill provider attempting to connect to Electron IPC..."); let path = desktop_core::ipc::path(IPC_PATH); @@ -451,8 +451,8 @@ fn send_message_helper( Ok(()) } -#[derive(Debug)] /// Types of errors for callbacks. +#[derive(Debug)] pub enum CallbackError { Timeout, Cancelled, @@ -583,6 +583,7 @@ impl PreparePasskeyRegistrationCallback for TimedCallback PathBuf { + static SERVER_COUNTER: AtomicU32 = AtomicU32::new(0); + let counter = SERVER_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let name = format!("{}-{}", IPC_PATH, counter); + desktop_core::ipc::path(&name) + } + + /// Sets up an in-memory server based on the passed handler and returns a client to the server. fn get_client< F: Fn(Result) -> Result + Send + 'static, >( handler: F, ) -> AutofillProviderClient { - static SERVER_COUNTER: AtomicU32 = AtomicU32::new(0); let (signal_tx, signal_rx) = std::sync::mpsc::channel(); - // use a counter to allow tests to run in parallel without interfering with each other. - let counter = SERVER_COUNTER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); - let name = format!("{}-{}", IPC_PATH, counter); - let path = desktop_core::ipc::path(&name); + let path = get_server_path(); let server_path = path.clone(); + + // Start server thread std::thread::spawn(move || { let _span = tracing::span!(Level::DEBUG, "server").entered(); - tracing::info!("[server] Starting server thread"); + tracing::info!("Starting server thread"); let (tx, mut rx) = mpsc::channel(8); let rt = tokio::runtime::Builder::new_current_thread() .enable_io() .build() .unwrap(); rt.block_on(async move { - tracing::debug!("[server] starting server at {server_path:?}"); + tracing::debug!("Starting server at {server_path:?}"); let server = desktop_core::ipc::server::Server::start(&server_path, tx).unwrap(); - tracing::debug!("[server] Server started"); + + // Signal to main thread that the server is ready to process messages. + tracing::debug!(" Server started"); signal_tx.send(()).unwrap(); - tracing::debug!("[server] Waiting for messages"); + + // Handle incoming messages + tracing::debug!("Waiting for messages"); while let Some(data) = rx.recv().await { + tracing::debug!("Received {data:?}"); match data.kind { - MessageType::Connected => tracing::debug!("[server] Connected"), - MessageType::Disconnected => tracing::debug!("[server] Disconnected"), + MessageType::Connected => {} + MessageType::Disconnected => {} MessageType::Message => { - tracing::debug!( - "[server] received {}", - data.message.as_ref().unwrap().to_string() - ); + // Deserialize and handle messages using the given handler function. let msg: SerializedMessage = serde_json::from_str(&data.message.unwrap()).unwrap(); @@ -652,38 +665,54 @@ mod tests { } }); }); - tracing::debug!("[client] waiting for server..."); + + // Wait for server to startup and client to connect to server before returning client to test method. + let _span = tracing::span!(Level::DEBUG, "client"); + tracing::debug!("Waiting for server..."); signal_rx.recv_timeout(Duration::from_millis(1000)).unwrap(); - tracing::debug!("[client] Starting client..."); + + // This starts a background task to connect to the server. + tracing::debug!("Starting client..."); let client = AutofillProviderClient::connect_to_path(path.to_path_buf()); - tracing::debug!("[client] Client connecting..."); - for _ in 0..10 { + + // The client connects to the server asynchronously in a background + // thread, so wait for client to report itself as Connected so that test + // methods don't have to do this everytime. + // Note, this has the potential to be flaky on a very busy server, but that's unavoidable with the current API. + tracing::debug!("Client connecting..."); + for _ in 0..20 { if let ConnectionStatus::Connected = client.get_connection_status() { break; } std::thread::sleep(Duration::from_millis(10)); } + assert!(matches!( client.get_connection_status(), ConnectionStatus::Connected )); + client } #[test] fn test_disconnected_gives_error() { - let client = AutofillProviderClient::connect(); + // There is no server running at this path, so this client should always be disconnected. + let client = AutofillProviderClient::connect_to_path(get_server_path()); + + // use an arbitrary request to test whether the client is disconnected. let callback = Arc::new(TimedCallback::new()); client.get_lock_status(callback.clone()); let response = callback .wait_for_response(Duration::from_millis(10), None) .unwrap(); + assert!(matches!(response, Err(BitwardenError::Disconnected))); } #[test] fn test_get_lock_status() { - // tracing_subscriber::fmt().init(); + // The server should expect a lock status request and return a valid response. let handler = |value: Result| { let value = value.unwrap(); if let Ok(LockStatusRequest {}) = serde_json::from_value(value.clone()) { @@ -694,6 +723,8 @@ mod tests { ))) } }; + + // send a lock status request let client = get_client(handler); let callback = Arc::new(TimedCallback::new()); client.get_lock_status(callback.clone()); @@ -701,6 +732,7 @@ mod tests { .wait_for_response(Duration::from_millis(3000), None) .unwrap() .unwrap(); + assert!(response.is_unlocked); } } diff --git a/apps/desktop/desktop_native/autofill_provider/src/lock_status.rs b/apps/desktop/desktop_native/autofill_provider/src/lock_status.rs index 134070bc54a..0c590ced15d 100644 --- a/apps/desktop/desktop_native/autofill_provider/src/lock_status.rs +++ b/apps/desktop/desktop_native/autofill_provider/src/lock_status.rs @@ -4,12 +4,12 @@ use serde::{Deserialize, Serialize}; use crate::{BitwardenError, Callback, TimedCallback}; -/// Request to retrieve the lock status of the desktop client. #[derive(Debug, Serialize, Deserialize)] +/// Request to retrieve the lock status of the desktop client. pub(super) struct LockStatusRequest {} -/// Response for the lock status of the desktop client. #[derive(Debug, Deserialize)] +/// Response for the lock status of the desktop client. pub struct LockStatusResponse { /// Whether the desktop client is unlocked. #[serde(rename = "isUnlocked")] diff --git a/apps/desktop/desktop_native/autofill_provider/src/registration.rs b/apps/desktop/desktop_native/autofill_provider/src/registration.rs index ff1c4930d35..83bc0088251 100644 --- a/apps/desktop/desktop_native/autofill_provider/src/registration.rs +++ b/apps/desktop/desktop_native/autofill_provider/src/registration.rs @@ -4,10 +4,10 @@ use serde::{Deserialize, Serialize}; use crate::{BitwardenError, Callback, Position, UserVerification}; +/// Request to create a credential. #[cfg_attr(target_os = "macos", derive(uniffi::Record))] #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -/// Request to create a credential. pub struct PasskeyRegistrationRequest { /// Relying Party ID for the request. pub rp_id: String, @@ -42,7 +42,6 @@ pub struct PasskeyRegistrationRequest { /// List of excluded credential IDs. pub excluded_credentials: Vec>, - #[cfg(not(target_os = "macos"))] /// Byte string representing the native OS window handle for the WebAuthn client. /// # Operating System Differences /// @@ -51,9 +50,9 @@ pub struct PasskeyRegistrationRequest { /// /// ## Windows /// On Windows, this is a HWND. + #[cfg(not(target_os = "macos"))] pub client_window_handle: Vec, - #[cfg(not(target_os = "macos"))] /// Native context required for callbacks to the OS. Format differs by OS. /// # Operating System Differences /// @@ -63,13 +62,14 @@ pub struct PasskeyRegistrationRequest { /// ## Windows /// On Windows, this is a base64-string representing the following data: /// `request transaction id (GUID, 16 bytes) || SHA-256(pluginOperationRequest)` + #[cfg(not(target_os = "macos"))] pub context: String, } +/// Response for a passkey registration request. #[cfg_attr(target_os = "macos", derive(uniffi::Record))] #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] -/// Response for a passkey registration request. pub struct PasskeyRegistrationResponse { /// Relying Party ID. pub rp_id: String, diff --git a/apps/desktop/desktop_native/autofill_provider/src/window_handle_query.rs b/apps/desktop/desktop_native/autofill_provider/src/window_handle_query.rs index 1b1f1a8d9cb..c98323fc04a 100644 --- a/apps/desktop/desktop_native/autofill_provider/src/window_handle_query.rs +++ b/apps/desktop/desktop_native/autofill_provider/src/window_handle_query.rs @@ -4,8 +4,8 @@ use serde::{Deserialize, Serialize}; use crate::{BitwardenError, Callback, TimedCallback}; -#[derive(Debug, Default, Serialize, Deserialize)] /// Request to get the window handle of the desktop client. +#[derive(Debug, Default, Serialize, Deserialize)] pub(super) struct WindowHandleQueryRequest { #[serde(rename = "windowHandle")] /// Marker field for parsing; data is never read. @@ -15,9 +15,9 @@ pub(super) struct WindowHandleQueryRequest { window_handle: String, } +/// Response to window handle request. #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] -/// Response to window handle request. pub struct WindowHandleQueryResponse { /// Whether the desktop client is currently visible. pub is_visible: bool,