From f035857d9a9dc14c85e6bdf22cbe72528235657d Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 7 Jun 2018 03:03:29 +0200 Subject: Use Result<(), CommandError> instead of CommandStatus The Result enum is more idiomatic and easier to use than our custom CommandStatus enum with the same structure. This is especially true for the try operator ?. --- src/tests/device.rs | 89 +++++++++++++++++++++++++++++------------------------ src/tests/otp.rs | 55 +++++++++++++++------------------ src/tests/pws.rs | 69 ++++++++++------------------------------- 3 files changed, 89 insertions(+), 124 deletions(-) (limited to 'src/tests') diff --git a/src/tests/device.rs b/src/tests/device.rs index 22423fa..60ca33a 100644 --- a/src/tests/device.rs +++ b/src/tests/device.rs @@ -2,7 +2,7 @@ use std::ffi::CStr; use std::process::Command; use std::{thread, time}; use tests::util::{Target, ADMIN_PASSWORD, USER_PASSWORD}; -use {Authenticate, CommandError, CommandStatus, Config, Device}; +use {Authenticate, CommandError, Config, Device, Storage}; static ADMIN_NEW_PASSWORD: &str = "1234567890"; static USER_NEW_PASSWORD: &str = "abcdefghij"; @@ -130,23 +130,20 @@ fn config() { let device = Target::connect().unwrap(); let admin = device.authenticate_admin(ADMIN_PASSWORD).unwrap(); let config = Config::new(None, None, None, true); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); let get_config = admin.get_config().unwrap(); assert_eq!(config, get_config); let config = Config::new(None, Some(9), None, true); - assert_eq!( - CommandStatus::Error(CommandError::InvalidSlot), - admin.write_config(config) - ); + assert_eq!(Err(CommandError::InvalidSlot), admin.write_config(config)); let config = Config::new(Some(1), None, Some(0), false); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); let get_config = admin.get_config().unwrap(); assert_eq!(config, get_config); let config = Config::new(None, None, None, false); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); let get_config = admin.get_config().unwrap(); assert_eq!(config, get_config); } @@ -158,8 +155,11 @@ fn change_user_pin() { let device = device.authenticate_user(USER_PASSWORD).unwrap().device(); let device = device.authenticate_user(USER_NEW_PASSWORD).unwrap_err().0; - let result = device.change_user_pin(USER_PASSWORD, USER_NEW_PASSWORD); - assert_eq!(CommandStatus::Success, result); + assert!( + device + .change_user_pin(USER_PASSWORD, USER_NEW_PASSWORD) + .is_ok() + ); let device = device.authenticate_user(USER_PASSWORD).unwrap_err().0; let device = device @@ -168,13 +168,16 @@ fn change_user_pin() { .device(); let result = device.change_user_pin(USER_PASSWORD, USER_PASSWORD); - assert_eq!(CommandStatus::Error(CommandError::WrongPassword), result); + assert_eq!(Err(CommandError::WrongPassword), result); - let result = device.change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD); - assert_eq!(CommandStatus::Success, result); + assert!( + device + .change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD) + .is_ok() + ); let device = device.authenticate_user(USER_PASSWORD).unwrap().device(); - device.authenticate_user(USER_NEW_PASSWORD).unwrap_err(); + assert!(device.authenticate_user(USER_NEW_PASSWORD).is_err()); } #[test] @@ -184,8 +187,11 @@ fn change_admin_pin() { let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap().device(); let device = device.authenticate_admin(ADMIN_NEW_PASSWORD).unwrap_err().0; - let result = device.change_admin_pin(ADMIN_PASSWORD, ADMIN_NEW_PASSWORD); - assert_eq!(CommandStatus::Success, result); + assert!( + device + .change_admin_pin(ADMIN_PASSWORD, ADMIN_NEW_PASSWORD) + .is_ok() + ); let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap_err().0; let device = device @@ -193,11 +199,16 @@ fn change_admin_pin() { .unwrap() .device(); - let result = device.change_admin_pin(ADMIN_PASSWORD, ADMIN_PASSWORD); - assert_eq!(CommandStatus::Error(CommandError::WrongPassword), result); + assert_eq!( + Err(CommandError::WrongPassword), + device.change_admin_pin(ADMIN_PASSWORD, ADMIN_PASSWORD) + ); - let result = device.change_admin_pin(ADMIN_NEW_PASSWORD, ADMIN_PASSWORD); - assert_eq!(CommandStatus::Success, result); + assert!( + device + .change_admin_pin(ADMIN_NEW_PASSWORD, ADMIN_PASSWORD) + .is_ok() + ); let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap().device(); device.authenticate_admin(ADMIN_NEW_PASSWORD).unwrap_err(); @@ -216,12 +227,13 @@ fn require_failed_user_login(device: Target, password: &str, error: CommandError fn unlock_user_pin() { let device = Target::connect().unwrap(); let device = device.authenticate_user(USER_PASSWORD).unwrap().device(); - assert_eq!( - CommandStatus::Success, - device.unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD) + assert!( + device + .unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD) + .is_ok() ); assert_eq!( - CommandStatus::Error(CommandError::WrongPassword), + Err(CommandError::WrongPassword), device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD) ); @@ -232,12 +244,13 @@ fn unlock_user_pin() { let device = require_failed_user_login(device, USER_PASSWORD, CommandError::WrongPassword); assert_eq!( - CommandStatus::Error(CommandError::WrongPassword), + Err(CommandError::WrongPassword), device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD) ); - assert_eq!( - CommandStatus::Success, - device.unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD) + assert!( + device + .unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD) + .is_ok() ); device.authenticate_user(USER_PASSWORD).unwrap(); } @@ -246,22 +259,19 @@ fn unlock_user_pin() { #[cfg_attr(not(feature = "test-storage"), ignore)] fn encrypted_volume() { let device = Storage::connect().unwrap(); - assert_eq!(CommandStatus::Success, device.lock()); + assert!(device.lock().is_ok()); assert_eq!(1, count_nitrokey_block_devices()); - assert_eq!(CommandStatus::Success, device.disable_encrypted_volume()); + assert!(device.disable_encrypted_volume().is_ok()); assert_eq!(1, count_nitrokey_block_devices()); assert_eq!( - CommandStatus::Error(CommandError::WrongPassword), + Err(CommandError::WrongPassword), device.enable_encrypted_volume("123") ); assert_eq!(1, count_nitrokey_block_devices()); - assert_eq!( - CommandStatus::Success, - device.enable_encrypted_volume(USER_PASSWORD) - ); + assert!(device.enable_encrypted_volume(USER_PASSWORD).is_ok()); assert_eq!(2, count_nitrokey_block_devices()); - assert_eq!(CommandStatus::Success, device.disable_encrypted_volume()); + assert!(device.disable_encrypted_volume().is_ok()); assert_eq!(1, count_nitrokey_block_devices()); } @@ -270,10 +280,7 @@ fn encrypted_volume() { fn lock() { let device = Storage::connect().unwrap(); - assert_eq!( - CommandStatus::Success, - device.enable_encrypted_volume(USER_PASSWORD) - ); - assert_eq!(CommandStatus::Success, device.lock()); + assert!(device.enable_encrypted_volume(USER_PASSWORD).is_ok()); + assert!(device.lock().is_ok()); assert_eq!(1, count_nitrokey_block_devices()); } diff --git a/src/tests/otp.rs b/src/tests/otp.rs index 91f9f1b..cf71d9d 100644 --- a/src/tests/otp.rs +++ b/src/tests/otp.rs @@ -1,7 +1,6 @@ use std::ops::Deref; use tests::util::{Target, ADMIN_PASSWORD, USER_PASSWORD}; -use {Admin, Authenticate, CommandError, CommandStatus, Config, ConfigureOtp, GenerateOtp, OtpMode, - OtpSlotData}; +use {Admin, Authenticate, CommandError, Config, ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData}; // test suite according to RFC 4226, Appendix D static HOTP_SECRET: &str = "3132333435363738393031323334353637383930"; @@ -36,10 +35,7 @@ fn get_admin_test_device() -> Admin { fn configure_hotp(admin: &ConfigureOtp, counter: u8) { let slot_data = OtpSlotData::new(1, "test-hotp", HOTP_SECRET, OtpMode::SixDigits); - assert_eq!( - CommandStatus::Success, - admin.write_hotp_slot(slot_data, counter.into()) - ); + assert!(admin.write_hotp_slot(slot_data, counter.into()).is_ok()); } fn check_hotp_codes(device: &GenerateOtp, offset: u8) { @@ -56,7 +52,7 @@ fn check_hotp_codes(device: &GenerateOtp, offset: u8) { fn hotp_no_pin() { let admin = get_admin_test_device(); let config = Config::new(None, None, None, false); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); configure_hotp(&admin, 0); check_hotp_codes(admin.deref(), 0); @@ -73,7 +69,7 @@ fn hotp_no_pin() { fn hotp_pin() { let admin = get_admin_test_device(); let config = Config::new(None, None, None, true); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); configure_hotp(&admin, 0); let user = admin.device().authenticate_user(USER_PASSWORD).unwrap(); @@ -87,7 +83,7 @@ fn hotp_pin() { fn hotp_slot_name() { let admin = get_admin_test_device(); let slot_data = OtpSlotData::new(1, "test-hotp", HOTP_SECRET, OtpMode::SixDigits); - assert_eq!(CommandStatus::Success, admin.write_hotp_slot(slot_data, 0)); + assert!(admin.write_hotp_slot(slot_data, 0).is_ok()); let device = admin.device(); let result = device.get_hotp_slot_name(1); @@ -102,12 +98,12 @@ fn hotp_error() { let admin = get_admin_test_device(); let slot_data = OtpSlotData::new(1, "", HOTP_SECRET, OtpMode::SixDigits); assert_eq!( - CommandStatus::Error(CommandError::NoName), + Err(CommandError::NoName), admin.write_hotp_slot(slot_data, 0) ); let slot_data = OtpSlotData::new(4, "test", HOTP_SECRET, OtpMode::SixDigits); assert_eq!( - CommandStatus::Error(CommandError::InvalidSlot), + Err(CommandError::InvalidSlot), admin.write_hotp_slot(slot_data, 0) ); let code = admin.get_hotp_code(4); @@ -119,13 +115,13 @@ fn hotp_error() { fn hotp_erase() { let admin = get_admin_test_device(); let config = Config::new(None, None, None, false); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); let slot_data = OtpSlotData::new(1, "test1", HOTP_SECRET, OtpMode::SixDigits); - assert_eq!(CommandStatus::Success, admin.write_hotp_slot(slot_data, 0)); + assert!(admin.write_hotp_slot(slot_data, 0).is_ok()); let slot_data = OtpSlotData::new(2, "test2", HOTP_SECRET, OtpMode::SixDigits); - assert_eq!(CommandStatus::Success, admin.write_hotp_slot(slot_data, 0)); + assert!(admin.write_hotp_slot(slot_data, 0).is_ok()); - assert_eq!(CommandStatus::Success, admin.erase_hotp_slot(1)); + assert!(admin.erase_hotp_slot(1).is_ok()); let device = admin.device(); let result = device.get_hotp_slot_name(1); @@ -139,10 +135,7 @@ fn hotp_erase() { fn configure_totp(admin: &ConfigureOtp, factor: u64) { let slot_data = OtpSlotData::new(1, "test-totp", TOTP_SECRET, OtpMode::EightDigits); let time_window = 30u64.checked_mul(factor).unwrap(); - assert_eq!( - CommandStatus::Success, - admin.write_totp_slot(slot_data, time_window as u16) - ); + assert!(admin.write_totp_slot(slot_data, time_window as u16).is_ok()); } fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimestampSize) { @@ -153,7 +146,7 @@ fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimes continue; } - assert_eq!(CommandStatus::Success, device.set_time(time)); + assert!(device.set_time(time).is_ok()); let result = device.get_totp_code(1); assert!(result.is_ok()); let result_code = result.unwrap(); @@ -171,7 +164,7 @@ fn totp_no_pin() { // TODO: this test may fail due to bad timing --> find solution let admin = get_admin_test_device(); let config = Config::new(None, None, None, false); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); configure_totp(&admin, 1); check_totp_codes(admin.deref(), 1, TotpTimestampSize::U32); @@ -191,7 +184,7 @@ fn totp_no_pin() { fn totp_no_pin_64() { let admin = get_admin_test_device(); let config = Config::new(None, None, None, false); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); configure_totp(&admin, 1); check_totp_codes(admin.deref(), 1, TotpTimestampSize::U64); @@ -209,7 +202,7 @@ fn totp_pin() { // TODO: this test may fail due to bad timing --> find solution let admin = get_admin_test_device(); let config = Config::new(None, None, None, true); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); configure_totp(&admin, 1); let user = admin.device().authenticate_user(USER_PASSWORD).unwrap(); @@ -225,7 +218,7 @@ fn totp_pin() { fn totp_pin_64() { let admin = get_admin_test_device(); let config = Config::new(None, None, None, true); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); configure_totp(&admin, 1); let user = admin.device().authenticate_user(USER_PASSWORD).unwrap(); @@ -239,7 +232,7 @@ fn totp_pin_64() { fn totp_slot_name() { let admin = get_admin_test_device(); let slot_data = OtpSlotData::new(1, "test-totp", TOTP_SECRET, OtpMode::EightDigits); - assert_eq!(CommandStatus::Success, admin.write_totp_slot(slot_data, 0)); + assert!(admin.write_totp_slot(slot_data, 0).is_ok()); let device = admin.device(); let result = device.get_totp_slot_name(1); @@ -255,12 +248,12 @@ fn totp_error() { let admin = get_admin_test_device(); let slot_data = OtpSlotData::new(1, "", HOTP_SECRET, OtpMode::SixDigits); assert_eq!( - CommandStatus::Error(CommandError::NoName), + Err(CommandError::NoName), admin.write_hotp_slot(slot_data, 0) ); let slot_data = OtpSlotData::new(4, "test", HOTP_SECRET, OtpMode::SixDigits); assert_eq!( - CommandStatus::Error(CommandError::InvalidSlot), + Err(CommandError::InvalidSlot), admin.write_hotp_slot(slot_data, 0) ); let code = admin.get_hotp_code(4); @@ -272,13 +265,13 @@ fn totp_error() { fn totp_erase() { let admin = get_admin_test_device(); let config = Config::new(None, None, None, false); - assert_eq!(CommandStatus::Success, admin.write_config(config)); + assert!(admin.write_config(config).is_ok()); let slot_data = OtpSlotData::new(1, "test1", TOTP_SECRET, OtpMode::SixDigits); - assert_eq!(CommandStatus::Success, admin.write_totp_slot(slot_data, 0)); + assert!(admin.write_totp_slot(slot_data, 0).is_ok()); let slot_data = OtpSlotData::new(2, "test2", TOTP_SECRET, OtpMode::SixDigits); - assert_eq!(CommandStatus::Success, admin.write_totp_slot(slot_data, 0)); + assert!(admin.write_totp_slot(slot_data, 0).is_ok()); - assert_eq!(CommandStatus::Success, admin.erase_totp_slot(1)); + assert!(admin.erase_totp_slot(1).is_ok()); let device = admin.device(); let result = device.get_totp_slot_name(1); diff --git a/src/tests/pws.rs b/src/tests/pws.rs index 02e33cd..f581515 100644 --- a/src/tests/pws.rs +++ b/src/tests/pws.rs @@ -2,7 +2,7 @@ use device::Device; use nitrokey_sys; use pws::{GetPasswordSafe, PasswordSafe, SLOT_COUNT}; use tests::util::{Target, ADMIN_PASSWORD, USER_PASSWORD}; -use util::{result_from_string, CommandError, CommandStatus}; +use util::{result_from_string, CommandError}; fn get_pws(device: &Target) -> PasswordSafe { device.get_password_safe(USER_PASSWORD).unwrap() @@ -28,17 +28,14 @@ fn drop() { let device = Target::connect().unwrap(); { let pws = get_pws(&device); - assert_eq!( - CommandStatus::Success, - pws.write_slot(1, "name", "login", "password") - ); + assert!(pws.write_slot(1, "name", "login", "password").is_ok()); assert_eq!("name", pws.get_slot_name(1).unwrap()); let result = result_from_string(unsafe { nitrokey_sys::NK_get_password_safe_slot_name(1) }); assert_eq!(Ok(String::from("name")), result); } let result = result_from_string(unsafe { nitrokey_sys::NK_get_password_safe_slot_name(1) }); assert_eq!(Ok(String::from("name")), result); - device.lock(); + assert!(device.lock().is_ok()); let result = result_from_string(unsafe { nitrokey_sys::NK_get_password_safe_slot_name(1) }); assert_eq!(Err(CommandError::NotAuthorized), result); } @@ -49,30 +46,19 @@ fn get_status() { let device = Target::connect().unwrap(); let pws = get_pws(&device); for i in 0..SLOT_COUNT { - assert_eq!( - CommandStatus::Success, - pws.erase_slot(i), - "Could not erase slot {}", - i - ); + assert!(pws.erase_slot(i).is_ok(), "Could not erase slot {}", i); } let status = pws.get_slot_status().unwrap(); assert_eq!(status, [false; SLOT_COUNT as usize]); - assert_eq!( - CommandStatus::Success, - pws.write_slot(1, "name", "login", "password") - ); + assert!(pws.write_slot(1, "name", "login", "password").is_ok()); let status = pws.get_slot_status().unwrap(); for i in 0..SLOT_COUNT { assert_eq!(i == 1, status[i as usize]); } for i in 0..SLOT_COUNT { - assert_eq!( - CommandStatus::Success, - pws.write_slot(i, "name", "login", "password") - ); + assert!(pws.write_slot(i, "name", "login", "password").is_ok()); } let status = pws.get_slot_status().unwrap(); assert_eq!(status, [true; SLOT_COUNT as usize]); @@ -83,15 +69,12 @@ fn get_status() { fn get_data() { let device = Target::connect().unwrap(); let pws = get_pws(&device); - assert_eq!( - CommandStatus::Success, - pws.write_slot(1, "name", "login", "password") - ); + assert!(pws.write_slot(1, "name", "login", "password").is_ok()); assert_eq!("name", pws.get_slot_name(1).unwrap()); assert_eq!("login", pws.get_slot_login(1).unwrap()); assert_eq!("password", pws.get_slot_password(1).unwrap()); - assert_eq!(CommandStatus::Success, pws.erase_slot(1)); + assert!(pws.erase_slot(1).is_ok()); // TODO: check error codes assert_eq!(Err(CommandError::Unknown), pws.get_slot_name(1)); assert_eq!(Err(CommandError::Unknown), pws.get_slot_login(1)); @@ -100,10 +83,7 @@ fn get_data() { let name = "with å"; let login = "pär@test.com"; let password = "'i3lJc[09?I:,[u7dWz9"; - assert_eq!( - CommandStatus::Success, - pws.write_slot(1, name, login, password) - ); + assert!(pws.write_slot(1, name, login, password).is_ok()); assert_eq!(name, pws.get_slot_name(1).unwrap()); assert_eq!(login, pws.get_slot_login(1).unwrap()); assert_eq!(password, pws.get_slot_password(1).unwrap()); @@ -129,30 +109,21 @@ fn write() { let pws = get_pws(&device); assert_eq!( - CommandStatus::Error(CommandError::InvalidSlot), + Err(CommandError::InvalidSlot), pws.write_slot(SLOT_COUNT, "name", "login", "password") ); - assert_eq!( - CommandStatus::Success, - pws.write_slot(0, "", "login", "password") - ); + assert!(pws.write_slot(0, "", "login", "password").is_ok()); assert_eq!(Err(CommandError::Unknown), pws.get_slot_name(0)); assert_eq!(Ok(String::from("login")), pws.get_slot_login(0)); assert_eq!(Ok(String::from("password")), pws.get_slot_password(0)); - assert_eq!( - CommandStatus::Success, - pws.write_slot(0, "name", "", "password") - ); + assert!(pws.write_slot(0, "name", "", "password").is_ok()); assert_eq!(Ok(String::from("name")), pws.get_slot_name(0)); assert_eq!(Err(CommandError::Unknown), pws.get_slot_login(0)); assert_eq!(Ok(String::from("password")), pws.get_slot_password(0)); - assert_eq!( - CommandStatus::Success, - pws.write_slot(0, "name", "login", "") - ); + assert!(pws.write_slot(0, "name", "login", "").is_ok()); assert_eq!(Ok(String::from("name")), pws.get_slot_name(0)); assert_eq!(Ok(String::from("login")), pws.get_slot_login(0)); assert_eq!(Err(CommandError::Unknown), pws.get_slot_password(0)); @@ -163,16 +134,10 @@ fn write() { fn erase() { let device = Target::connect().unwrap(); let pws = get_pws(&device); - assert_eq!( - CommandStatus::Error(CommandError::InvalidSlot), - pws.erase_slot(SLOT_COUNT) - ); + assert_eq!(Err(CommandError::InvalidSlot), pws.erase_slot(SLOT_COUNT)); - assert_eq!( - CommandStatus::Success, - pws.write_slot(0, "name", "login", "password") - ); - assert_eq!(CommandStatus::Success, pws.erase_slot(0)); - assert_eq!(CommandStatus::Success, pws.erase_slot(0)); + assert!(pws.write_slot(0, "name", "login", "password").is_ok()); + assert!(pws.erase_slot(0).is_ok()); + assert!(pws.erase_slot(0).is_ok()); assert_eq!(Err(CommandError::Unknown), pws.get_slot_name(0)); } -- cgit v1.2.1