From 1f8e482cfebb13b64002d65e61e29932770388be Mon Sep 17 00:00:00 2001
From: Robin Krahl <robin.krahl@ireas.org>
Date: Sat, 25 Jan 2020 21:07:07 +0100
Subject: Refactor connection handling

This patch introduces two new functions, find_device and connect, to
connect to a Nitrokey device.  find_device queries the attached Nitrokey
devices, applies the filters (currently only the --model option) and
returns the first match.  connect calls find_device and connects to the
returned device.
This refactoring allows us to add more device filters, for example a
--serial-number option, without code duplication.
---
 src/commands.rs     | 59 +++++++++++++++++++++++++++++++++++++++++++----------
 src/main.rs         |  1 +
 src/tests/status.rs | 14 +++++++------
 3 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/src/commands.rs b/src/commands.rs
index 67d230e..ff95c31 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -6,6 +6,7 @@
 use std::convert::TryFrom as _;
 use std::fmt;
 use std::mem;
+use std::ops::Deref as _;
 use std::thread;
 use std::time;
 use std::u8;
@@ -20,6 +21,7 @@ use nitrokey::GenerateOtp;
 use nitrokey::GetPasswordSafe;
 
 use crate::args;
+use crate::config;
 use crate::pinentry;
 use crate::Context;
 
@@ -39,6 +41,45 @@ fn set_log_level(ctx: &mut Context<'_>) {
   nitrokey::set_log_level(log_lvl);
 }
 
+/// Create a filter string from the program configuration.
+fn format_filter(config: &config::Config) -> String {
+  if let Some(model) = config.model {
+    format!(" (filter: model={})", model.as_ref())
+  } else {
+    String::new()
+  }
+}
+
+/// Find a Nitrokey device that matches the given requirements
+fn find_device(config: &config::Config) -> anyhow::Result<nitrokey::DeviceInfo> {
+  let devices = nitrokey::list_devices().context("Failed to enumerate Nitrokey devices")?;
+  let nkmodel = config.model.map(nitrokey::Model::from);
+  let mut iter = devices
+    .into_iter()
+    .filter(|device| nkmodel.is_none() || device.model == nkmodel);
+
+  let device = iter
+    .next()
+    .with_context(|| format!("Nitrokey device not found{}", format_filter(config)))?;
+  Ok(device)
+}
+
+/// Connect to a Nitrokey device that matches the given requirements
+fn connect<'mgr>(
+  manager: &'mgr mut nitrokey::Manager,
+  config: &config::Config,
+) -> anyhow::Result<nitrokey::DeviceWrapper<'mgr>> {
+  let device_info = find_device(config)?;
+  manager
+    .connect_path(device_info.path.deref())
+    .with_context(|| {
+      format!(
+        "Failed to connect to Nitrokey device at path {}",
+        device_info.path
+      )
+    })
+}
+
 /// Connect to any Nitrokey device and do something with it.
 fn with_device<F>(ctx: &mut Context<'_>, op: F) -> anyhow::Result<()>
 where
@@ -49,13 +90,7 @@ where
 
   set_log_level(ctx);
 
-  let device = match ctx.config.model {
-    Some(model) => manager.connect_model(model.into()).with_context(|| {
-      anyhow::anyhow!("Nitrokey {} device not found", model.as_user_facing_str())
-    })?,
-    None => manager.connect().context("Nitrokey device not found")?,
-  };
-
+  let device = connect(&mut manager, &ctx.config)?;
   op(ctx, device)
 }
 
@@ -75,10 +110,12 @@ where
     }
   }
 
-  let device = manager
-    .connect_storage()
-    .context("Nitrokey Storage device not found")?;
-  op(ctx, device)
+  let device = connect(&mut manager, &ctx.config)?;
+  if let nitrokey::DeviceWrapper::Storage(storage) = device {
+    op(ctx, storage)
+  } else {
+    panic!("connect returned a wrong model: {:?}", device)
+  }
 }
 
 /// Connect to any Nitrokey device, retrieve a password safe handle, and
diff --git a/src/main.rs b/src/main.rs
index baad15c..16715f9 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -3,6 +3,7 @@
 // Copyright (C) 2017-2020 The Nitrocli Developers
 // SPDX-License-Identifier: GPL-3.0-or-later
 
+#![allow(clippy::trivially_copy_pass_by_ref)]
 #![warn(
   bad_style,
   dead_code,
diff --git a/src/tests/status.rs b/src/tests/status.rs
index d158103..7946929 100644
--- a/src/tests/status.rs
+++ b/src/tests/status.rs
@@ -11,12 +11,7 @@ fn not_found_raw() {
 
   assert_ne!(rc, 0);
   assert_eq!(out, b"");
-  let expected = r#"Nitrokey device not found
-
-Caused by:
-    Communication error: Could not connect to a Nitrokey device
-"#;
-  assert_eq!(err, expected.as_bytes());
+  assert_eq!(err, b"Nitrokey device not found\n");
 }
 
 #[test_device]
@@ -26,6 +21,13 @@ fn not_found() {
   assert_eq!(err, "Nitrokey device not found");
 }
 
+#[test_device]
+fn not_found_pro() {
+  let res = Nitrocli::new().handle(&["status", "--model=pro"]);
+  let err = res.unwrap_err().to_string();
+  assert_eq!(err, "Nitrokey device not found (filter: model=pro)");
+}
+
 #[test_device(pro)]
 fn output_pro(model: nitrokey::Model) -> anyhow::Result<()> {
   let re = regex::Regex::new(
-- 
cgit v1.2.1