aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobin Krahl <robin.krahl@ireas.org>2019-01-17 14:21:44 +0000
committerRobin Krahl <robin.krahl@ireas.org>2019-01-20 22:15:29 +0100
commitd87859975dc158919ecd5bf11a1111a2da5fcb30 (patch)
tree27eb8ac46b130e08d917a7b91da8b48a14b644b6
parent17f9c30a0ace070cba856e4e89fcccedcab5e8e6 (diff)
downloadnitrokey-rs-d87859975dc158919ecd5bf11a1111a2da5fcb30.tar.gz
nitrokey-rs-d87859975dc158919ecd5bf11a1111a2da5fcb30.tar.bz2
Check specific error codes in the tests
If possible, check specific error codes instead of `is_err()`. This makes the code more readable and catches bugs resulting in the wrong error code. Also, using the assert_*_err and assert_ok macros yields error messages containing the expected and the actual value. To be able to use these macros with the `get_password_safe` method, we also have to implement `Debug` for `PasswordSafe` and `Device`.
-rw-r--r--TODO.md1
-rw-r--r--src/device.rs2
-rw-r--r--src/pws.rs1
-rw-r--r--tests/device.rs46
-rw-r--r--tests/otp.rs20
-rw-r--r--tests/pws.rs6
-rw-r--r--tests/util/mod.rs7
7 files changed, 32 insertions, 51 deletions
diff --git a/TODO.md b/TODO.md
index 53de7e9..487f56d 100644
--- a/TODO.md
+++ b/TODO.md
@@ -9,7 +9,6 @@
- Clear passwords from memory.
- Find a nicer syntax for the `write_config` test.
- Prevent construction of internal types.
-- More specific error checking in the tests.
- Check integer conversions.
- Consider implementing `Into<CommandError>` for `(Device, CommandError)`
- Lock password safe in `PasswordSafe::drop()` (see [nitrokey-storage-firmware
diff --git a/src/device.rs b/src/device.rs
index 1cf9da9..16064c3 100644
--- a/src/device.rs
+++ b/src/device.rs
@@ -286,7 +286,7 @@ pub struct StorageStatus {
///
/// This trait provides the commands that can be executed without authentication and that are
/// present on all supported Nitrokey devices.
-pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp {
+pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp + fmt::Debug {
/// Returns the model of the connected Nitrokey device.
///
/// # Example
diff --git a/src/pws.rs b/src/pws.rs
index 47965d7..a21527c 100644
--- a/src/pws.rs
+++ b/src/pws.rs
@@ -52,6 +52,7 @@ pub const SLOT_COUNT: u8 = 16;
/// [`get_password_safe`]: trait.GetPasswordSafe.html#method.get_password_safe
/// [`lock`]: trait.Device.html#method.lock
/// [`GetPasswordSafe`]: trait.GetPasswordSafe.html
+#[derive(Debug)]
pub struct PasswordSafe<'a> {
_device: &'a dyn Device,
}
diff --git a/tests/device.rs b/tests/device.rs
index ee5dae1..c502945 100644
--- a/tests/device.rs
+++ b/tests/device.rs
@@ -5,8 +5,8 @@ use std::process::Command;
use std::{thread, time};
use nitrokey::{
- Authenticate, CommandError, Config, ConfigureOtp, Device, Error, GenerateOtp, GetPasswordSafe,
- LibraryError, OtpMode, OtpSlotData, Storage, VolumeMode,
+ Authenticate, CommandError, CommunicationError, Config, ConfigureOtp, Device, Error,
+ GenerateOtp, GetPasswordSafe, LibraryError, OtpMode, OtpSlotData, Storage, VolumeMode,
};
use nitrokey_test::test as test_device;
@@ -31,11 +31,11 @@ fn count_nitrokey_block_devices() -> usize {
#[test_device]
fn connect_no_device() {
- assert!(nitrokey::connect().is_err());
- assert!(nitrokey::connect_model(nitrokey::Model::Pro).is_err());
- assert!(nitrokey::connect_model(nitrokey::Model::Storage).is_err());
- assert!(nitrokey::Pro::connect().is_err());
- assert!(nitrokey::Storage::connect().is_err());
+ assert_cmu_err!(CommunicationError::NotConnected, nitrokey::connect());
+ assert_cmu_err!(CommunicationError::NotConnected, nitrokey::connect_model(nitrokey::Model::Pro));
+ assert_cmu_err!(CommunicationError::NotConnected, nitrokey::connect_model(nitrokey::Model::Storage));
+ assert_cmu_err!(CommunicationError::NotConnected, nitrokey::Pro::connect());
+ assert_cmu_err!(CommunicationError::NotConnected, nitrokey::Storage::connect());
}
#[test_device]
@@ -148,9 +148,7 @@ fn change_user_pin(device: DeviceWrapper) {
let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
let device = device.authenticate_user(USER_NEW_PASSWORD).unwrap_err().0;
- assert!(device
- .change_user_pin(USER_PASSWORD, USER_NEW_PASSWORD)
- .is_ok());
+ assert_ok!((), device.change_user_pin(USER_PASSWORD, USER_NEW_PASSWORD));
let device = device.authenticate_user(USER_PASSWORD).unwrap_err().0;
let device = device
@@ -161,9 +159,7 @@ fn change_user_pin(device: DeviceWrapper) {
let result = device.change_user_pin(USER_PASSWORD, USER_PASSWORD);
assert_cmd_err!(CommandError::WrongPassword, result);
- assert!(device
- .change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD)
- .is_ok());
+ assert_ok!((), device.change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD));
let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
assert!(device.authenticate_user(USER_NEW_PASSWORD).is_err());
@@ -174,9 +170,7 @@ fn change_admin_pin(device: DeviceWrapper) {
let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap().device();
let device = device.authenticate_admin(ADMIN_NEW_PASSWORD).unwrap_err().0;
- assert!(device
- .change_admin_pin(ADMIN_PASSWORD, ADMIN_NEW_PASSWORD)
- .is_ok());
+ assert_ok!((), device.change_admin_pin(ADMIN_PASSWORD, ADMIN_NEW_PASSWORD));
let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap_err().0;
let device = device
@@ -189,9 +183,7 @@ fn change_admin_pin(device: DeviceWrapper) {
device.change_admin_pin(ADMIN_PASSWORD, ADMIN_PASSWORD)
);
- assert!(device
- .change_admin_pin(ADMIN_NEW_PASSWORD, ADMIN_PASSWORD)
- .is_ok());
+ assert_ok!((), device.change_admin_pin(ADMIN_NEW_PASSWORD, ADMIN_PASSWORD));
let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap().device();
device.authenticate_admin(ADMIN_NEW_PASSWORD).unwrap_err();
@@ -215,9 +207,7 @@ where
#[test_device]
fn unlock_user_pin(device: DeviceWrapper) {
let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
- assert!(device
- .unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD)
- .is_ok());
+ assert_ok!((), device.unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD));
assert_cmd_err!(
CommandError::WrongPassword,
device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD)
@@ -235,9 +225,7 @@ fn unlock_user_pin(device: DeviceWrapper) {
CommandError::WrongPassword,
device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD)
);
- assert!(device
- .unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD)
- .is_ok());
+ assert_ok!((), device.unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD));
let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
// block user PIN
@@ -251,14 +239,10 @@ fn unlock_user_pin(device: DeviceWrapper) {
CommandError::WrongPassword,
device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD)
);
- assert!(device
- .unlock_user_pin(ADMIN_PASSWORD, USER_NEW_PASSWORD)
- .is_ok());
+ assert_ok!((), device.unlock_user_pin(ADMIN_PASSWORD, USER_NEW_PASSWORD));
// reset user PIN
- assert!(device
- .change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD)
- .is_ok());
+ assert_ok!((), device.change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD));
}
#[test_device]
diff --git a/tests/otp.rs b/tests/otp.rs
index 51a6539..96da371 100644
--- a/tests/otp.rs
+++ b/tests/otp.rs
@@ -93,7 +93,7 @@ fn hotp_pin(device: DeviceWrapper) {
let user = admin.device().authenticate_user(USER_PASSWORD).unwrap();
check_hotp_codes(&user, 0);
- assert!(user.device().get_hotp_code(1).is_err());
+ assert_cmd_err!(CommandError::NotAuthorized, user.device().get_hotp_code(1));
}
#[test_device]
@@ -156,7 +156,7 @@ fn configure_totp(admin: &ConfigureOtp, factor: u64) {
}
fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimestampSize) {
- for (i, &(base_time, code)) in TOTP_CODES.iter().enumerate() {
+ for (base_time, code) in TOTP_CODES {
let time = base_time.checked_mul(factor).unwrap();
let is_u64 = time > u32::max_value() as u64;
if is_u64 != (timestamp_size == TotpTimestampSize::U64) {
@@ -164,14 +164,7 @@ fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimes
}
assert_ok!((), device.set_time(time, true));
- let result = device.get_totp_code(1);
- assert!(result.is_ok());
- let result_code = result.unwrap();
- assert_eq!(
- code, result_code,
- "TOTP code {} should be {} but is {}",
- i, code, result_code
- );
+ assert_ok!(code.to_string(), device.get_totp_code(1));
}
}
@@ -221,7 +214,7 @@ fn totp_pin(device: DeviceWrapper) {
let user = admin.device().authenticate_user(USER_PASSWORD).unwrap();
check_totp_codes(&user, 1, TotpTimestampSize::U32);
- assert!(user.device().get_totp_code(1).is_err());
+ assert_cmd_err!(CommandError::NotAuthorized, user.device().get_totp_code(1));
}
#[test_device]
@@ -235,7 +228,7 @@ fn totp_pin_64(device: Pro) {
let user = admin.device().authenticate_user(USER_PASSWORD).unwrap();
check_totp_codes(&user, 1, TotpTimestampSize::U64);
- assert!(user.device().get_totp_code(1).is_err());
+ assert_cmd_err!(CommandError::NotAuthorized, user.device().get_totp_code(1));
}
#[test_device]
@@ -246,8 +239,7 @@ fn totp_slot_name(device: DeviceWrapper) {
let device = admin.device();
let result = device.get_totp_slot_name(1);
- assert!(result.is_ok());
- assert_eq!("test-totp", result.unwrap());
+ assert_ok!("test-totp", result);
let result = device.get_totp_slot_name(16);
assert_lib_err!(LibraryError::InvalidSlot, result);
}
diff --git a/tests/pws.rs b/tests/pws.rs
index b89d7f6..7a97983 100644
--- a/tests/pws.rs
+++ b/tests/pws.rs
@@ -39,11 +39,9 @@ where
#[test_device]
fn enable(device: DeviceWrapper) {
- assert!(device
- .get_password_safe(&(USER_PASSWORD.to_owned() + "123"))
- .is_err());
+ assert_cmd_err!(CommandError::WrongPassword, device.get_password_safe(&(USER_PASSWORD.to_owned() + "123")));
assert!(device.get_password_safe(USER_PASSWORD).is_ok());
- assert!(device.get_password_safe(ADMIN_PASSWORD).is_err());
+ assert_cmd_err!(CommandError::WrongPassword, device.get_password_safe(ADMIN_PASSWORD));
assert!(device.get_password_safe(USER_PASSWORD).is_ok());
}
diff --git a/tests/util/mod.rs b/tests/util/mod.rs
index b1d3ea3..4a00a66 100644
--- a/tests/util/mod.rs
+++ b/tests/util/mod.rs
@@ -69,6 +69,13 @@ macro_rules! assert_cmd_err {
}
#[macro_export]
+macro_rules! assert_cmu_err {
+ ($left:expr, $right:expr) => {
+ assert_err!(::nitrokey::Error::CommunicationError, $left, $right);
+ };
+}
+
+#[macro_export]
macro_rules! assert_lib_err {
($left:expr, $right:expr) => {
assert_err!(::nitrokey::Error::LibraryError, $left, $right);