Implement new windows focus behavior

apps/desktop/desktop_native/Cargo.lock

@@ -764,7 +764,6 @@ dependencies = [
  "pkcs8",
  "rand",
  "rand_chacha",
- "retry",
  "rsa",
  "russh-cryptovec",
  "scopeguard",
@@ -2106,15 +2105,6 @@ version = "0.8.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "2b15c43186be67a4fd63bee50d0303afffcef381492ebe2c5d87f324e1b8815c"
 
-[[package]]
-name = "retry"
-version = "2.0.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9166d72162de3575f950507683fac47e30f6f2c3836b71b7fbc61aa517c9c5f4"
-dependencies = [
- "rand",
-]
-
 [[package]]
 name = "rsa"
 version = "0.9.6"

apps/desktop/desktop_native/core/Cargo.toml

@@ -6,18 +6,16 @@ version = "0.0.0"
 publish = false
 
 [features]
-default = ["sys"]
-manual_test = []
-
-sys = [
+default = [
   "dep:widestring",
   "dep:windows",
   "dep:core-foundation",
   "dep:security-framework",
   "dep:security-framework-sys",
   "dep:zbus",
-  "dep:zbus_polkit",
+  "dep:zbus_polkit"
 ]
+manual_test = []
 
 [dependencies]
 aes = "=0.8.4"
@@ -38,7 +36,6 @@ futures = "=0.3.31"
 interprocess = { version = "=2.2.1", features = ["tokio"] }
 log = "=0.4.22"
 rand = "=0.8.5"
-retry = "=2.0.0"
 russh-cryptovec = "=0.7.3"
 scopeguard = "=1.2.0"
 sha2 = "=0.10.8"

apps/desktop/desktop_native/core/src/biometric/mod.rs

@@ -2,12 +2,16 @@ use aes::cipher::generic_array::GenericArray;
 use anyhow::{anyhow, Result};
 
 #[cfg_attr(target_os = "linux", path = "unix.rs")]
-#[cfg_attr(target_os = "windows", path = "windows.rs")]
 #[cfg_attr(target_os = "macos", path = "macos.rs")]
+#[cfg_attr(target_os = "windows", path = "windows.rs")]
 mod biometric;
 
-use base64::{engine::general_purpose::STANDARD as base64_engine, Engine};
 pub use biometric::Biometric;
+
+#[cfg(target_os = "windows")]
+pub mod windows_focus;
+
+use base64::{engine::general_purpose::STANDARD as base64_engine, Engine};
 use sha2::{Digest, Sha256};
 
 use crate::crypto::{self, CipherString};

apps/desktop/desktop_native/core/src/biometric/windows.rs

@@ -1,12 +1,15 @@
-use std::{ffi::c_void, str::FromStr};
+use std::{
+    ffi::c_void,
+    str::FromStr,
+    sync::{atomic::AtomicBool, Arc},
+};
 
 use anyhow::{anyhow, Result};
 use base64::{engine::general_purpose::STANDARD as base64_engine, Engine};
 use rand::RngCore;
-use retry::delay::Fixed;
 use sha2::{Digest, Sha256};
 use windows::{
-    core::{factory, h, s, HSTRING},
+    core::{factory, h, HSTRING},
     Foundation::IAsyncOperation,
     Security::{
         Credentials::{
@@ -17,13 +20,6 @@ use windows::{
     Win32::{
         Foundation::HWND,
         System::WinRT::IUserConsentVerifierInterop,
-        UI::{
-            Input::KeyboardAndMouse::{
-                keybd_event, GetAsyncKeyState, SetFocus, KEYEVENTF_EXTENDEDKEY, KEYEVENTF_KEYUP,
-                VK_MENU,
-            },
-            WindowsAndMessaging::{FindWindowA, SetForegroundWindow},
-        },
     },
 };
 
@@ -32,7 +28,7 @@ use crate::{
     crypto::CipherString,
 };
 
-use super::{decrypt, encrypt};
+use super::{decrypt, encrypt, windows_focus::{focus_security_prompt, set_focus}};
 
 /// The Windows OS implementation of the biometric trait.
 pub struct Biometric {}
@@ -103,8 +99,22 @@ impl super::BiometricTrait for Biometric {
 
         let challenge_buffer = CryptographicBuffer::CreateFromByteArray(&challenge)?;
         let async_operation = result.Credential()?.RequestSignAsync(&challenge_buffer)?;
-        focus_security_prompt()?;
-        let signature = async_operation.get()?;
+        focus_security_prompt();
+
+        let done = Arc::new(AtomicBool::new(false));
+        let done_clone = done.clone();
+        let _ = tokio::task::spawn_blocking(move || loop {
+            if !done_clone.load(std::sync::atomic::Ordering::Relaxed) {
+                focus_security_prompt();
+                std::thread::sleep(std::time::Duration::from_millis(500));
+            } else {
+                break;
+            }
+        });
+
+        let signature = async_operation.get();
+        done.store(true, std::sync::atomic::Ordering::Relaxed);
+        let signature = signature?;
 
         if signature.Status()? != KeyCredentialStatus::Success {
             return Err(anyhow!("Failed to sign data"));
@@ -168,56 +178,6 @@ fn random_challenge() -> [u8; 16] {
     challenge
 }
 
-/// Searches for a window that looks like a security prompt and set it as focused.
-///
-/// Gives up after 1.5 seconds with a delay of 500ms between each try.
-fn focus_security_prompt() -> Result<()> {
-    unsafe fn try_find_and_set_focus(
-        class_name: windows::core::PCSTR,
-    ) -> retry::OperationResult<(), ()> {
-        let hwnd = unsafe { FindWindowA(class_name, None) };
-        if let Ok(hwnd) = hwnd {
-            set_focus(hwnd);
-            return retry::OperationResult::Ok(());
-        }
-        retry::OperationResult::Retry(())
-    }
-
-    let class_name = s!("Credential Dialog Xaml Host");
-    retry::retry_with_index(Fixed::from_millis(500), |current_try| {
-        if current_try > 3 {
-            return retry::OperationResult::Err(());
-        }
-
-        unsafe { try_find_and_set_focus(class_name) }
-    })
-    .map_err(|_| anyhow!("Failed to find security prompt"))
-}
-
-fn set_focus(window: HWND) {
-    let mut pressed = false;
-
-    unsafe {
-        // Simulate holding down Alt key to bypass windows limitations
-        //  https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getasynckeystate#return-value
-        //  The most significant bit indicates if the key is currently being pressed. This means the
-        //  value will be negative if the key is pressed.
-        if GetAsyncKeyState(VK_MENU.0 as i32) >= 0 {
-            pressed = true;
-            keybd_event(VK_MENU.0 as u8, 0, KEYEVENTF_EXTENDEDKEY, 0);
-        }
-        SetForegroundWindow(window);
-        SetFocus(window);
-        if pressed {
-            keybd_event(
-                VK_MENU.0 as u8,
-                0,
-                KEYEVENTF_EXTENDEDKEY | KEYEVENTF_KEYUP,
-                0,
-            );
-        }
-    }
-}
 
 #[cfg(test)]
 mod tests {
@@ -310,12 +270,16 @@ mod tests {
             os_key_part_b64: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned(),
             client_key_part_b64: Some("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned()),
         };
-        crate::password::set_password(test, test, secret).await.unwrap();
+        crate::password::set_password(test, test, secret)
+            .await
+            .unwrap();
         let result =
             <Biometric as BiometricTrait>::get_biometric_secret(test, test, Some(key_material))
                 .await
                 .unwrap();
-        crate::password::delete_password("test", "test").await.unwrap();
+        crate::password::delete_password("test", "test")
+            .await
+            .unwrap();
         assert_eq!(result, secret);
     }
 
@@ -328,19 +292,24 @@ mod tests {
             os_key_part_b64: "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned(),
             client_key_part_b64: Some("AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=".to_owned()),
         };
-        crate::password::set_password(test, test, &secret.to_string()).await.unwrap();
+        crate::password::set_password(test, test, &secret.to_string())
+            .await
+            .unwrap();
 
         let result =
             <Biometric as BiometricTrait>::get_biometric_secret(test, test, Some(key_material))
                 .await
                 .unwrap();
-        crate::password::delete_password("test", "test").await.unwrap();
+        crate::password::delete_password("test", "test")
+            .await
+            .unwrap();
         assert_eq!(result, "secret");
     }
 
     #[tokio::test]
     async fn set_biometric_secret_requires_key() {
-        let result = <Biometric as BiometricTrait>::set_biometric_secret("", "", "", None, "").await;
+        let result =
+            <Biometric as BiometricTrait>::set_biometric_secret("", "", "", None, "").await;
         assert!(result.is_err());
         assert_eq!(
             result.unwrap_err().to_string(),

apps/desktop/desktop_native/core/src/biometric/windows_focus.rs

@@ -0,0 +1,19 @@
+use windows::{core::s, Win32::{Foundation::HWND, UI::{Input::KeyboardAndMouse::SetFocus, WindowsAndMessaging::{FindWindowA, SetForegroundWindow}}}};
+
+/// Searches for a window that looks like a security prompt and set it as focused.
+/// Only works when the process has permission to foreground, either by being in foreground
+/// Or by being given foreground permission https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setforegroundwindow#remarks
+pub fn focus_security_prompt() -> () {
+    let class_name = s!("Credential Dialog Xaml Host");
+    let hwnd = unsafe { FindWindowA(class_name, None) };
+    if let Ok(hwnd) = hwnd {
+        set_focus(hwnd);
+    }
+}
+
+pub(crate) fn set_focus(window: HWND) {
+    unsafe {
+        let _ = SetForegroundWindow(window);
+        let _ = SetFocus(window);
+    }
+}

apps/desktop/desktop_native/core/src/lib.rs

@@ -1,16 +1,10 @@
 pub mod autofill;
-#[cfg(feature = "sys")]
 pub mod biometric;
-#[cfg(feature = "sys")]
 pub mod clipboard;
 pub mod crypto;
 pub mod error;
 pub mod ipc;
-#[cfg(feature = "sys")]
 pub mod password;
-#[cfg(feature = "sys")]
 pub mod powermonitor;
-#[cfg(feature = "sys")]
 pub mod process_isolation;
-#[cfg(feature = "sys")]
 pub mod ssh_agent;

apps/desktop/desktop_native/proxy/Cargo.toml

@@ -8,7 +8,7 @@ publish = false
 
 [dependencies]
 anyhow = "=1.0.94"
-desktop_core = { path = "../core", default-features = false }
+desktop_core = { path = "../core" }
 futures = "=0.3.31"
 log = "=0.4.22"
 simplelog = "=0.12.2"

apps/desktop/desktop_native/proxy/src/main.rs

@@ -5,6 +5,9 @@ use futures::{FutureExt, SinkExt, StreamExt};
 use log::*;
 use tokio_util::codec::LengthDelimitedCodec;
 
+#[cfg(target_os = "windows")]
+mod windows;
+
 #[cfg(target_os = "macos")]
 embed_plist::embed_info_plist!("../../../resources/info.desktop_proxy.plist");
 
@@ -49,6 +52,9 @@ fn init_logging(log_path: &Path, console_level: LevelFilter, file_level: LevelFi
 ///
 #[tokio::main(flavor = "current_thread")]
 async fn main() {
+    #[cfg(target_os = "windows")]
+    let should_foreground = windows::allow_foreground();
+
     let sock_path = desktop_core::ipc::path("bitwarden");
 
     let log_path = {
@@ -142,6 +148,9 @@ async fn main() {
 
             // Listen to stdin and send messages to ipc processor.
             msg = stdin.next() => {
+                #[cfg(target_os = "windows")]
+                should_foreground.store(true, std::sync::atomic::Ordering::Relaxed);
+
                 match msg {
                     Some(Ok(msg)) => {
                         let m = String::from_utf8(msg.to_vec()).unwrap();

apps/desktop/desktop_native/proxy/src/windows.rs

@@ -0,0 +1,23 @@
+use std::sync::{
+    atomic::{AtomicBool, Ordering},
+    Arc,
+};
+
+pub fn allow_foreground() -> Arc<AtomicBool> {
+    let should_foreground = Arc::new(AtomicBool::new(false));
+    let should_foreground_clone = should_foreground.clone();
+    let _ = std::thread::spawn(move || loop {
+        if !should_foreground_clone.load(Ordering::Relaxed) {
+            std::thread::sleep(std::time::Duration::from_millis(100));
+            continue;
+        }
+        should_foreground_clone.store(false, Ordering::Relaxed);
+
+        for _ in 0..60 {
+            desktop_core::biometric::windows_focus::focus_security_prompt();
+            std::thread::sleep(std::time::Duration::from_millis(1000));
+        }
+    });
+
+    should_foreground
+}