refactor: improve UV module code quality and maintainability

- Use thiserror derive for VerificationError (removes boilerplate)
- Define priority constants module (FPRINTD=100, FACE=90, COMMAND=50, NOTIFICATION=10)
- Add config validation for face threshold (0.0-1.0 range)
- Cache tokio runtime in FprintdProvider (reuse across verify calls)
- Add from_config factory methods to all providers
- Simplify build_uv_manager with extend pattern

Author: pando85

Cargo.lock

@@ -42,6 +42,7 @@ zeroize.workspace = true
 hex.workspace = true
 shadow-rs.workspace = true
 dirs.workspace = true
+thiserror.workspace = true
 hmac = "0.12"
 rpassword = "7.3"
 atty = "0.2"

cmd/passless/Cargo.toml

@@ -2,8 +2,8 @@ use crate::pin_storage::PinStorage;
 use crate::storage::{CredentialFilter, CredentialStorage};
 use crate::util::bytes_to_hex;
 use crate::uv::{
-    FprintdProvider, NotificationProvider, UserVerificationManager, UserVerificationProvider,
-    VerificationContext, VerificationResult,
+    CommandProvider, FprintdProvider, NotificationProvider, UserVerificationManager,
+    UserVerificationProvider, VerificationContext, VerificationResult,
 };
 
 #[cfg(feature = "face")]
@@ -141,49 +141,37 @@ fn build_uv_manager(config: &UvConfig) -> UserVerificationManager {
 
     match config.provider {
         UvProviderType::Auto => {
-            if config.fprintd.enabled {
-                providers.push(Box::new(FprintdProvider::new()));
-            }
+            providers.extend(FprintdProvider::from_config(config.fprintd.enabled));
             #[cfg(feature = "face")]
-            if config.face.enabled {
-                providers.push(Box::new(FaceIdProvider::new(
-                    config.face.camera_index,
-                    config.face.threshold,
-                    None,
-                )));
-            }
-            providers.push(Box::new(NotificationProvider::new(config.timeout_seconds)));
+            providers.extend(FaceIdProvider::from_config(
+                config.face.enabled,
+                config.face.camera_index,
+                config.face.threshold,
+            ));
+            providers.push(NotificationProvider::from_config(config.timeout_seconds));
         }
         UvProviderType::Fprintd => {
-            if config.fprintd.enabled {
-                providers.push(Box::new(FprintdProvider::new()));
-            }
+            providers.extend(FprintdProvider::from_config(config.fprintd.enabled));
         }
         UvProviderType::Face => {
             #[cfg(feature = "face")]
-            if config.face.enabled {
-                providers.push(Box::new(FaceIdProvider::new(
-                    config.face.camera_index,
-                    config.face.threshold,
-                    None,
-                )));
-            }
+            providers.extend(FaceIdProvider::from_config(
+                config.face.enabled,
+                config.face.camera_index,
+                config.face.threshold,
+            ));
             #[cfg(not(feature = "face"))]
-            {
-                log::warn!("Face provider requested but 'face' feature not enabled");
-            }
+            log::warn!("Face provider requested but 'face' feature not enabled");
         }
         UvProviderType::Notification => {
-            providers.push(Box::new(NotificationProvider::new(config.timeout_seconds)));
+            providers.push(NotificationProvider::from_config(config.timeout_seconds));
         }
         UvProviderType::Command => {
-            if config.command.enabled && !config.command.command.is_empty() {
-                use crate::uv::CommandProvider;
-                providers.push(Box::new(CommandProvider::new(
-                    config.command.command.clone(),
-                    config.timeout_seconds,
-                )));
-            }
+            providers.extend(CommandProvider::from_config(
+                config.command.enabled,
+                config.command.command.clone(),
+                config.timeout_seconds,
+            ));
         }
     }
 

cmd/passless/src/authenticator.rs

@@ -242,6 +242,12 @@ fn run() -> Result<()> {
     // Load config: CLI args + config file + defaults (CLI takes precedence)
     let config = AppConfig::load(&mut args);
 
+    // Validate configuration
+    if let Err(e) = config.validate() {
+        error!("Configuration validation failed: {}", e);
+        process::exit(1);
+    }
+
     if config.verbose && log_level != log::LevelFilter::Debug {
         info!("Enabling verbose logging...");
         log::set_max_level(log::LevelFilter::Debug);

cmd/passless/src/main.rs

@@ -16,7 +16,9 @@
 //! - `PASSLESS_UV_USER`: User identifier (if available)
 //! - `PASSLESS_UV_TIMEOUT`: Timeout in seconds
 
-use super::{UserVerificationProvider, VerificationContext, VerificationError, VerificationResult};
+use super::{
+    UserVerificationProvider, VerificationContext, VerificationError, VerificationResult, priority,
+};
 
 use log::{debug, info};
 
@@ -43,6 +45,19 @@ impl CommandProvider {
         }
     }
 
+    /// Create a provider if enabled and configured
+    pub fn from_config(
+        enabled: bool,
+        command: Vec<String>,
+        timeout_seconds: u32,
+    ) -> Option<Box<dyn UserVerificationProvider>> {
+        if enabled && !command.is_empty() {
+            Some(Box::new(Self::new(command, timeout_seconds)))
+        } else {
+            None
+        }
+    }
+
     /// Execute the verification command
     fn execute(
         &self,
@@ -129,7 +144,7 @@ impl UserVerificationProvider for CommandProvider {
     }
 
     fn priority(&self) -> u8 {
-        50
+        priority::COMMAND
     }
 }
 
@@ -146,7 +161,7 @@ mod tests {
     #[test]
     fn test_command_provider_priority() {
         let provider = CommandProvider::new(vec!["echo".to_string()], 30);
-        assert_eq!(provider.priority(), 50);
+        assert_eq!(provider.priority(), priority::COMMAND);
     }
 
     #[test]

cmd/passless/src/uv/command.rs

@@ -13,7 +13,9 @@
 //! During enrollment, multiple face embeddings are captured and averaged
 //! to create a robust template. The template is stored securely.
 
-use super::{UserVerificationProvider, VerificationContext, VerificationError, VerificationResult};
+use super::{
+    UserVerificationProvider, VerificationContext, VerificationError, VerificationResult, priority,
+};
 
 use log::{debug, info, warn};
 
@@ -58,6 +60,19 @@ impl FaceIdProvider {
         }
     }
 
+    /// Create a provider if enabled in config
+    pub fn from_config(
+        enabled: bool,
+        camera_index: usize,
+        threshold: f32,
+    ) -> Option<Box<dyn UserVerificationProvider>> {
+        if enabled {
+            Some(Box::new(Self::new(camera_index, threshold, None)))
+        } else {
+            None
+        }
+    }
+
     /// Get path to the stored embeddings file
     fn embeddings_file(&self) -> PathBuf {
         self.embeddings_path.join("face_template.bin")
@@ -275,7 +290,7 @@ impl UserVerificationProvider for FaceIdProvider {
     }
 
     fn priority(&self) -> u8 {
-        90
+        priority::FACE
     }
 
     fn supports_enrollment(&self) -> bool {
@@ -358,7 +373,7 @@ mod tests {
     #[test]
     fn test_face_provider_priority() {
         let provider = FaceIdProvider::new(0, DEFAULT_THRESHOLD, None);
-        assert_eq!(provider.priority(), 90);
+        assert_eq!(provider.priority(), priority::FACE);
     }
 
     #[test]

cmd/passless/src/uv/face.rs

@@ -8,25 +8,50 @@
 //! - A fingerprint reader must be connected
 //! - User must have enrolled fingerprints
 
-use super::{UserVerificationProvider, VerificationContext, VerificationError, VerificationResult};
+use super::{
+    UserVerificationProvider, VerificationContext, VerificationError, VerificationResult, priority,
+};
 
 use log::{debug, error, info, warn};
 
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
+use std::sync::Mutex;
 
 /// fprintd-based fingerprint verification provider
 pub struct FprintdProvider {
-    available_cache: Arc<Mutex<Option<bool>>>,
+    available_cache: Mutex<Option<bool>>,
+    runtime: Mutex<Option<Arc<tokio::runtime::Runtime>>>,
 }
 
 impl FprintdProvider {
     /// Create a new fprintd provider
     pub fn new() -> Self {
         Self {
-            available_cache: Arc::new(Mutex::new(None)),
+            available_cache: Mutex::new(None),
+            runtime: Mutex::new(None),
         }
     }
 
+    /// Create a provider if enabled in config
+    pub fn from_config(enabled: bool) -> Option<Box<dyn UserVerificationProvider>> {
+        if enabled {
+            Some(Box::new(Self::new()))
+        } else {
+            None
+        }
+    }
+
+    /// Get or create the tokio runtime
+    fn get_runtime(&self) -> Result<Arc<tokio::runtime::Runtime>, VerificationError> {
+        let mut runtime_guard = self.runtime.lock().unwrap();
+        if runtime_guard.is_none() {
+            *runtime_guard = Some(Arc::new(tokio::runtime::Runtime::new().map_err(|e| {
+                VerificationError::DeviceError(format!("Failed to create runtime: {}", e))
+            })?));
+        }
+        Ok(runtime_guard.as_ref().unwrap().clone())
+    }
+
     async fn do_verify(timeout_secs: u32) -> Result<VerificationResult, VerificationError> {
         use zbus::Connection;
 
@@ -195,8 +220,8 @@ impl FprintdProvider {
         Ok(result)
     }
 
-    fn check_available_sync() -> bool {
-        let rt = match tokio::runtime::Runtime::new() {
+    fn check_available(&self) -> bool {
+        let rt = match self.get_runtime() {
             Ok(rt) => rt,
             Err(_) => return false,
         };
@@ -252,7 +277,7 @@ impl UserVerificationProvider for FprintdProvider {
             return cached;
         }
 
-        let available = Self::check_available_sync();
+        let available = self.check_available();
         *self.available_cache.lock().unwrap() = Some(available);
         available
     }
@@ -262,22 +287,13 @@ impl UserVerificationProvider for FprintdProvider {
         context: &VerificationContext,
     ) -> Result<VerificationResult, VerificationError> {
         let timeout_secs = context.timeout_seconds;
+        let rt = self.get_runtime()?;
 
-        let result = std::thread::spawn(move || {
-            let rt = tokio::runtime::Runtime::new().map_err(|e| {
-                VerificationError::DeviceError(format!("Failed to create runtime: {}", e))
-            })?;
-
-            rt.block_on(async { Self::do_verify(timeout_secs).await })
-        })
-        .join()
-        .map_err(|_| VerificationError::Other("Verification thread panicked".into()))??;
-
-        Ok(result)
+        rt.block_on(async { Self::do_verify(timeout_secs).await })
     }
 
     fn priority(&self) -> u8 {
-        100
+        priority::FPRINTD
     }
 
     fn supports_enrollment(&self) -> bool {
@@ -306,7 +322,7 @@ mod tests {
     #[test]
     fn test_fprintd_provider_priority() {
         let provider = FprintdProvider::new();
-        assert_eq!(provider.priority(), 100);
+        assert_eq!(provider.priority(), priority::FPRINTD);
     }
 
     #[test]