From 4a7ce051bd4004fb62f1c7022d92efa2ce42b6ab Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 13 Jan 2019 12:03:34 +0100 Subject: Change Device::enumerate return type to use DeviceInfo The return type of Device::enumerate is changed from std::vector to std::vector to expose the additional information contained in the DeviceInfo struct. --- unittest/test_multiple_devices.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'unittest') diff --git a/unittest/test_multiple_devices.cc b/unittest/test_multiple_devices.cc index cd78681..b224653 100644 --- a/unittest/test_multiple_devices.cc +++ b/unittest/test_multiple_devices.cc @@ -37,7 +37,8 @@ TEST_CASE("List devices", "[BASIC]") { shared_ptr d = make_shared(); auto v = d->enumerate(); REQUIRE(v.size() > 0); - for (auto a : v){ + for (auto i : v){ + auto a = i.m_path; std::cout << a; d->set_path(a); d->connect(); @@ -57,7 +58,8 @@ TEST_CASE("Regenerate AES keys", "[BASIC]") { REQUIRE(v.size() > 0); std::vector> devices; - for (auto a : v){ + for (auto i : v){ + auto a = i.m_path; std::cout << a << endl; d = make_shared(); d->set_path(a); -- cgit v1.2.3 From 71d4ecc04c23342f207e7f1133ea8824a1dcdd16 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 13 Jan 2019 12:03:43 +0100 Subject: Change Nitrokey::list_devices return type to use DeviceInfo In the previous commit, we changed the return value of Device::enumerate to std::vector. Now we change Nitrokey::list_devices to also return DeviceInfo instances. --- NitrokeyManager.cc | 9 ++------- libnitrokey/NitrokeyManager.h | 2 +- unittest/test_multiple_devices.cc | 5 ++++- 3 files changed, 7 insertions(+), 9 deletions(-) (limited to 'unittest') diff --git a/NitrokeyManager.cc b/NitrokeyManager.cc index 8825fce..8ca4698 100644 --- a/NitrokeyManager.cc +++ b/NitrokeyManager.cc @@ -105,16 +105,11 @@ using nitrokey::misc::strcpyT; return true; } - std::vector NitrokeyManager::list_devices(){ + std::vector NitrokeyManager::list_devices(){ std::lock_guard lock(mex_dev_com_manager); auto p = make_shared(); - auto device_infos = p->enumerate(); - std::vector strings; - strings.resize(device_infos.size()); - std::transform(device_infos.begin(), device_infos.end(), strings.begin(), - [](auto device_info) { return device_info.m_path; }); - return strings; + return p->enumerate(); } std::vector NitrokeyManager::list_devices_by_cpuID(){ diff --git a/libnitrokey/NitrokeyManager.h b/libnitrokey/NitrokeyManager.h index d6e5df4..6908143 100644 --- a/libnitrokey/NitrokeyManager.h +++ b/libnitrokey/NitrokeyManager.h @@ -80,7 +80,7 @@ char * strndup(const char* str, size_t maxlen); bool get_time(uint64_t time = 0); bool erase_totp_slot(uint8_t slot_number, const char *temporary_password); bool erase_hotp_slot(uint8_t slot_number, const char *temporary_password); - std::vector list_devices(); + std::vector list_devices(); std::vector list_devices_by_cpuID(); /** diff --git a/unittest/test_multiple_devices.cc b/unittest/test_multiple_devices.cc index b224653..f9e9ad2 100644 --- a/unittest/test_multiple_devices.cc +++ b/unittest/test_multiple_devices.cc @@ -90,7 +90,10 @@ TEST_CASE("Use API", "[BASIC]") { REQUIRE(v.size() > 0); for (int i=0; i<10; i++){ - for (auto a : v) { + for (auto i : v) { + if (i.m_deviceModel != DeviceModel::STORAGE) + continue; + auto a = i.m_path; std::cout <<"Connect with: " << a << " " << std::boolalpha << nm->connect_with_path(a) << " "; try{ -- cgit v1.2.3 From 1751759356bd64cc78f8f71543c3edd5cdce8376 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 13 Jan 2019 12:04:22 +0100 Subject: Make Device::enumerate static Device::enumerate does not need any instance data, therefore it is made static. Note that this not only changes the public API by making the method static. We also return all connected Nitrokey devices instead of only Storage devices. The NitrokeyManager method list_devices_by_cpuID is changed to check the device type so that they still only return Storage devices. The list_device method now returns both Storage and Pro devices. --- NitrokeyManager.cc | 10 +++++----- device.cc | 15 ++++++++------- libnitrokey/device.h | 5 ++--- unittest/test_multiple_devices.cc | 8 ++++++-- 4 files changed, 21 insertions(+), 17 deletions(-) (limited to 'unittest') diff --git a/NitrokeyManager.cc b/NitrokeyManager.cc index 8ca4698..3b57ba6 100644 --- a/NitrokeyManager.cc +++ b/NitrokeyManager.cc @@ -108,8 +108,7 @@ using nitrokey::misc::strcpyT; std::vector NitrokeyManager::list_devices(){ std::lock_guard lock(mex_dev_com_manager); - auto p = make_shared(); - return p->enumerate(); + return Device::enumerate(); } std::vector NitrokeyManager::list_devices_by_cpuID(){ @@ -127,12 +126,13 @@ using nitrokey::misc::strcpyT; LOGD1("Enumerating devices"); std::vector res; - auto d = make_shared(); - const auto v = d->enumerate(); + const auto v = Device::enumerate(); LOGD1("Discovering IDs"); for (auto & i: v){ + if (i.m_deviceModel != DeviceModel::STORAGE) + continue; auto p = i.m_path; - d = make_shared(); + auto d = make_shared(); LOGD1( std::string("Found: ") + p ); d->set_path(p); try{ diff --git a/device.cc b/device.cc index d3f6e09..58dc0e5 100644 --- a/device.cc +++ b/device.cc @@ -188,16 +188,17 @@ int Device::recv(void *packet) { } std::vector Device::enumerate(){ - //TODO make static - auto pInfo = hid_enumerate(m_vid, m_pid); + auto pInfo = hid_enumerate(NITROKEY_VID, 0); auto pInfo_ = pInfo; std::vector res; while (pInfo != nullptr){ - std::string path(pInfo->path); - std::wstring serialNumber(pInfo->serial_number); - auto deviceModel = this->get_device_model(); - DeviceInfo info = { deviceModel, path, serialNumber }; - res.push_back(info); + auto deviceModel = product_id_to_model(pInfo->product_id); + if (deviceModel.has_value()) { + std::string path(pInfo->path); + std::wstring serialNumber(pInfo->serial_number); + DeviceInfo info = { deviceModel.value(), path, serialNumber }; + res.push_back(info); + } pInfo = pInfo->next; } diff --git a/libnitrokey/device.h b/libnitrokey/device.h index 8fbb385..418d335 100644 --- a/libnitrokey/device.h +++ b/libnitrokey/device.h @@ -148,12 +148,11 @@ public: */ bool could_be_enumerated(); /** - * Returns a vector with all connected Nitrokey devices of the same device - * type as this device. + * Returns a vector with all connected Nitrokey devices. * * @return information about all connected devices */ - std::vector enumerate(); + static std::vector enumerate(); void show_stats(); diff --git a/unittest/test_multiple_devices.cc b/unittest/test_multiple_devices.cc index f9e9ad2..183af4f 100644 --- a/unittest/test_multiple_devices.cc +++ b/unittest/test_multiple_devices.cc @@ -35,9 +35,11 @@ using namespace nitrokey; TEST_CASE("List devices", "[BASIC]") { shared_ptr d = make_shared(); - auto v = d->enumerate(); + auto v = Device::enumerate(); REQUIRE(v.size() > 0); for (auto i : v){ + if (i.m_deviceModel != DeviceModel::STORAGE) + continue; auto a = i.m_path; std::cout << a; d->set_path(a); @@ -54,11 +56,13 @@ TEST_CASE("List devices", "[BASIC]") { TEST_CASE("Regenerate AES keys", "[BASIC]") { shared_ptr d = make_shared(); - auto v = d->enumerate(); + auto v = Device::enumerate(); REQUIRE(v.size() > 0); std::vector> devices; for (auto i : v){ + if (i.m_deviceModel != DeviceModel::STORAGE) + continue; auto a = i.m_path; std::cout << a << endl; d = make_shared(); -- cgit v1.2.3 From 4f7c1b31191d98904276fecd236e6b68b405c349 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 13 Jan 2019 12:04:51 +0100 Subject: Add test case for Device::enumerate to test_multiple_devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current test case is renamed to “List Storage devices” as it also displays the SD card ID. The new test case, “List devices”, lists all connected devices and prints their model, path and serial number. --- unittest/test_multiple_devices.cc | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'unittest') diff --git a/unittest/test_multiple_devices.cc b/unittest/test_multiple_devices.cc index 183af4f..2ea8a20 100644 --- a/unittest/test_multiple_devices.cc +++ b/unittest/test_multiple_devices.cc @@ -34,6 +34,28 @@ using namespace nitrokey; TEST_CASE("List devices", "[BASIC]") { + auto v = Device::enumerate(); + REQUIRE(v.size() > 0); + for (auto i : v){ + auto d = Device::create(i.m_deviceModel); + if (!d) { + std::cout << "Could not create device with model " << i.m_deviceModel << "\n"; + continue; + } + std::cout << i.m_deviceModel << " " << i.m_path << " "; + std::wcout << i.m_serialNumber; + std::cout << " |"; + d->set_path(i.m_path); + d->connect(); + auto res = GetStatus::CommandTransaction::run(d); + std::cout << " " << res.data().card_serial_u32 << " " + << res.data().get_card_serial_hex() + << std::endl; + d->disconnect(); + } +} + +TEST_CASE("List Storage devices", "[BASIC]") { shared_ptr d = make_shared(); auto v = Device::enumerate(); REQUIRE(v.size() > 0); -- cgit v1.2.3 From a63968570dc99ddf193589270218e6c6df34c460 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 13 Jan 2019 12:05:09 +0100 Subject: Add test case for NitrokeyManager::connect_with_path to test_multiple_devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current test case is renamed to “Use Storage API” as it queries the storage status. The new test case, “Use API”, lists all connected devices and prints their model, path and serial number. --- unittest/test_multiple_devices.cc | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'unittest') diff --git a/unittest/test_multiple_devices.cc b/unittest/test_multiple_devices.cc index 2ea8a20..d644cfd 100644 --- a/unittest/test_multiple_devices.cc +++ b/unittest/test_multiple_devices.cc @@ -115,6 +115,29 @@ TEST_CASE("Use API", "[BASIC]") { auto v = nm->list_devices(); REQUIRE(v.size() > 0); + for (auto i : v) { + std::cout << "Connect with: " << i.m_deviceModel << " " << i.m_path << " "; + std::wcout << i.m_serialNumber; + std::cout << " | " << std::boolalpha << nm->connect_with_path(i.m_path) << " |"; + try { + auto status = nm->get_status(); + std::cout << " " << status.card_serial_u32 << " " + << status.get_card_serial_hex() + << std::endl; + } catch (const LongOperationInProgressException &e) { + std::cout << "long operation in progress on " << i.m_path + << " " << std::to_string(e.progress_bar_value) << std::endl; + } + } +} + + +TEST_CASE("Use Storage API", "[BASIC]") { + auto nm = NitrokeyManager::instance(); + nm->set_loglevel(2); + auto v = nm->list_devices(); + REQUIRE(v.size() > 0); + for (int i=0; i<10; i++){ for (auto i : v) { if (i.m_deviceModel != DeviceModel::STORAGE) -- cgit v1.2.3 From a80378e0c770a503ddaafc0c7aacb78cac667b8f Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 13 Jan 2019 12:05:28 +0100 Subject: Change std::wstring to std::string in DeviceInfo For easier handling, we should use a std::string instead of std::wstring for the serial number in DeviceInfo. For the conversion, I assume that the serial number is valid UTF-8. As it should be alphanumeric and ASCII only, this should be true. --- device.cc | 6 +++++- libnitrokey/device.h | 2 +- unittest/test_multiple_devices.cc | 10 ++++------ 3 files changed, 10 insertions(+), 8 deletions(-) (limited to 'unittest') diff --git a/device.cc b/device.cc index 35aefca..bc42965 100644 --- a/device.cc +++ b/device.cc @@ -20,7 +20,9 @@ */ #include +#include #include +#include #include #include #include @@ -210,7 +212,9 @@ std::vector Device::enumerate(){ auto deviceModel = product_id_to_model(pInfo->product_id); if (deviceModel.has_value()) { std::string path(pInfo->path); - std::wstring serialNumber(pInfo->serial_number); + std::wstring serialNumberW(pInfo->serial_number); + std::wstring_convert> converter; + std::string serialNumber = converter.to_bytes(serialNumberW); DeviceInfo info = { deviceModel.value(), path, serialNumber }; res.push_back(info); } diff --git a/libnitrokey/device.h b/libnitrokey/device.h index 4b1c239..d50080d 100644 --- a/libnitrokey/device.h +++ b/libnitrokey/device.h @@ -92,7 +92,7 @@ struct DeviceInfo { /** * The serial number of the device. */ - std::wstring m_serialNumber; + std::string m_serialNumber; }; #include diff --git a/unittest/test_multiple_devices.cc b/unittest/test_multiple_devices.cc index d644cfd..bc1b60b 100644 --- a/unittest/test_multiple_devices.cc +++ b/unittest/test_multiple_devices.cc @@ -42,9 +42,8 @@ TEST_CASE("List devices", "[BASIC]") { std::cout << "Could not create device with model " << i.m_deviceModel << "\n"; continue; } - std::cout << i.m_deviceModel << " " << i.m_path << " "; - std::wcout << i.m_serialNumber; - std::cout << " |"; + std::cout << i.m_deviceModel << " " << i.m_path << " " + << i.m_serialNumber << " |"; d->set_path(i.m_path); d->connect(); auto res = GetStatus::CommandTransaction::run(d); @@ -116,9 +115,8 @@ TEST_CASE("Use API", "[BASIC]") { REQUIRE(v.size() > 0); for (auto i : v) { - std::cout << "Connect with: " << i.m_deviceModel << " " << i.m_path << " "; - std::wcout << i.m_serialNumber; - std::cout << " | " << std::boolalpha << nm->connect_with_path(i.m_path) << " |"; + std::cout << "Connect with: " << i.m_deviceModel << " " << i.m_path << " " + << i.m_serialNumber << " | " << std::boolalpha << nm->connect_with_path(i.m_path) << " |"; try { auto status = nm->get_status(); std::cout << " " << status.card_serial_u32 << " " -- cgit v1.2.3 From cf32902131d4f7bd68622ca9d243fdff5a5ed519 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 13 Jan 2019 12:05:55 +0100 Subject: Add simple test for NK_list_devices Unfortunately, I cannot test more as the current ffi implementation does not allow me to import struct definitions. Without the definition for the NK_device_info struct, I cannot inspect the result of the NK_list_devices function. --- unittest/test_multiple.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'unittest') diff --git a/unittest/test_multiple.py b/unittest/test_multiple.py index 3f1d2d5..902234f 100644 --- a/unittest/test_multiple.py +++ b/unittest/test_multiple.py @@ -29,11 +29,19 @@ from tqdm import tqdm from conftest import skip_if_device_version_lower_than from constants import DefaultPasswords, DeviceErrorCode, bb -from misc import gs, wait +from misc import gs, wait, ffi pprint = pprint.PrettyPrinter(indent=4).pprint +@pytest.mark.other +@pytest.mark.info +def test_list_devices(C): + infos = C.NK_list_devices() + assert infos != ffi.NULL + C.NK_free_device_info(infos) + + @pytest.mark.other @pytest.mark.info def test_get_status_storage_multiple(C): -- cgit v1.2.3 From e9cfa720d39e0c540fe530f907a84e9a4b5d1240 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 13 Jan 2019 12:06:16 +0100 Subject: Add test for NK_connect_with_path As we cannot read the output of NK_list_devices in the Python tests at the moment, this test case uses NK_list_devices_by_cpuID instead. --- unittest/test_multiple.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'unittest') diff --git a/unittest/test_multiple.py b/unittest/test_multiple.py index 902234f..821a3b7 100644 --- a/unittest/test_multiple.py +++ b/unittest/test_multiple.py @@ -42,6 +42,25 @@ def test_list_devices(C): C.NK_free_device_info(infos) +@pytest.mark.other +@pytest.mark.info +def test_connect_with_path(C): + ids = gs(C.NK_list_devices_by_cpuID()) + # NK_list_devices_by_cpuID already opened the devices, so we have to close + # them before trying to reconnect + assert C.NK_logout() == 0 + + devices_list = ids.split(b';') + for value in devices_list: + parts = value.split(b'_p_') + assert len(parts) < 3 + if len(parts) == 2: + path = parts[1] + else: + path = parts[0] + assert C.NK_connect_with_path(path) == 1 + + @pytest.mark.other @pytest.mark.info def test_get_status_storage_multiple(C): -- cgit v1.2.3 From cd1cfdbfc4113186f80dbadf5eb76543b22e34bd Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 13 Jan 2019 12:29:58 +0100 Subject: Add test for NK_list_devices to test_multiple_devices As we cannot test this function properly in Python (due to missing struct definitions), we test it in the C++ test suite. --- unittest/test_multiple_devices.cc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'unittest') diff --git a/unittest/test_multiple_devices.cc b/unittest/test_multiple_devices.cc index bc1b60b..4b1e2c1 100644 --- a/unittest/test_multiple_devices.cc +++ b/unittest/test_multiple_devices.cc @@ -29,6 +29,7 @@ const char * RFC_SECRET = "12345678901234567890"; #include #include #include +#include "../NK_C_API.h" using namespace nitrokey; @@ -108,6 +109,24 @@ TEST_CASE("Regenerate AES keys", "[BASIC]") { } +TEST_CASE("Use C API", "[BASIC]") { + auto ptr = NK_list_devices(); + auto first_ptr = ptr; + REQUIRE(ptr != nullptr); + + while (ptr) { + std::cout << "Connect with: " << ptr->model << " " << ptr->path << " " + << ptr->serial_number << " | " << NK_connect_with_path(ptr->path) << " | "; + auto status = NK_status(); + std::cout << status << std::endl; + free(status); + ptr = ptr->next; + } + + NK_free_device_info(first_ptr); +} + + TEST_CASE("Use API", "[BASIC]") { auto nm = NitrokeyManager::instance(); nm->set_loglevel(2); -- cgit v1.2.3 From ad6ed36c82cbdf6ebb244b93991d3187188f2b7d Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 13 Jan 2019 13:32:40 +0100 Subject: Add tqdm to unittest/requirements.txt The test_mulitple.py unit test requires tqdm, which is not a dependency of any of the requirements and was not included in the requirements list. --- unittest/requirements.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'unittest') diff --git a/unittest/requirements.txt b/unittest/requirements.txt index 5c0110b..6d718ad 100644 --- a/unittest/requirements.txt +++ b/unittest/requirements.txt @@ -1,4 +1,5 @@ cffi pytest-repeat pytest-randomly -oath \ No newline at end of file +tqdm +oath -- cgit v1.2.3