From 8efda62509a43edd03ca53f63c981b7d8c642dcb Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 10:32:51 +0000 Subject: [PATCH 01/18] test: [#1231] add tests for InMemoryKeyRepository --- packages/tracker-core/.gitignore | 1 + .../key/repository/in_memory.rs | 103 ++++++++++++++++++ 2 files changed, 104 insertions(+) create mode 100644 packages/tracker-core/.gitignore diff --git a/packages/tracker-core/.gitignore b/packages/tracker-core/.gitignore new file mode 100644 index 00000000..c5cb1afa --- /dev/null +++ b/packages/tracker-core/.gitignore @@ -0,0 +1 @@ +.coverage \ No newline at end of file diff --git a/packages/tracker-core/src/authentication/key/repository/in_memory.rs b/packages/tracker-core/src/authentication/key/repository/in_memory.rs index 41d34604..0a2fc50c 100644 --- a/packages/tracker-core/src/authentication/key/repository/in_memory.rs +++ b/packages/tracker-core/src/authentication/key/repository/in_memory.rs @@ -39,3 +39,106 @@ impl InMemoryKeyRepository { } } } + +#[cfg(test)] +mod tests { + + mod the_in_memory_key_repository_should { + use std::time::Duration; + + use crate::authentication::key::repository::in_memory::InMemoryKeyRepository; + use crate::authentication::key::Key; + use crate::authentication::PeerKey; + + #[tokio::test] + async fn insert_a_new_peer_key() { + let repository = InMemoryKeyRepository::default(); + + let new_peer_key = PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: Some(Duration::new(9999, 0)), + }; + + repository.insert(&new_peer_key).await; + + let peer_key = repository.get(&new_peer_key.key).await; + + assert_eq!(peer_key, Some(new_peer_key)); + } + + #[tokio::test] + async fn remove_a_new_peer_key() { + let repository = InMemoryKeyRepository::default(); + + let new_peer_key = PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: Some(Duration::new(9999, 0)), + }; + + repository.insert(&new_peer_key).await; + + repository.remove(&new_peer_key.key).await; + + let peer_key = repository.get(&new_peer_key.key).await; + + assert_eq!(peer_key, None); + } + + #[tokio::test] + async fn get_a_new_peer_key_by_its_internal_key() { + let repository = InMemoryKeyRepository::default(); + + let expected_peer_key = PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: Some(Duration::new(9999, 0)), + }; + + repository.insert(&expected_peer_key).await; + + let peer_key = repository.get(&expected_peer_key.key).await; + + assert_eq!(peer_key, Some(expected_peer_key)); + } + + #[tokio::test] + async fn clear_all_peer_keys() { + let repository = InMemoryKeyRepository::default(); + + let new_peer_key = PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: Some(Duration::new(9999, 0)), + }; + + repository.insert(&new_peer_key).await; + + repository.clear().await; + + let peer_key = repository.get(&new_peer_key.key).await; + + assert_eq!(peer_key, None); + } + + #[tokio::test] + async fn reset_the_peer_keys_with_a_new_list_of_keys() { + let repository = InMemoryKeyRepository::default(); + + let old_peer_key = PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: Some(Duration::new(9999, 0)), + }; + + repository.insert(&old_peer_key).await; + + let new_peer_key = PeerKey { + key: Key::new("kqdVKHlKKWXzAideqI5gvjBP4jdbe5dW").unwrap(), + valid_until: Some(Duration::new(9999, 0)), + }; + + repository.reset_with(vec![new_peer_key.clone()]).await; + + let peer_key = repository.get(&new_peer_key.key).await; + + assert_eq!(peer_key, Some(new_peer_key)); + } + } +} From 04ee425fcc6f69b84bc2ca02d69ad41c9cfd8c41 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 11:18:56 +0000 Subject: [PATCH 02/18] chore: add todo --- packages/tracker-core/src/databases/setup.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/tracker-core/src/databases/setup.rs b/packages/tracker-core/src/databases/setup.rs index 728913e0..8d24c63b 100644 --- a/packages/tracker-core/src/databases/setup.rs +++ b/packages/tracker-core/src/databases/setup.rs @@ -11,6 +11,8 @@ use super::Database; /// Will panic if database cannot be initialized. #[must_use] pub fn initialize_database(config: &Configuration) -> Arc> { + // todo: inject only core configuration + let driver = match config.core.database.driver { database::Driver::Sqlite3 => Driver::Sqlite3, database::Driver::MySQL => Driver::MySQL, From 7c8d2941f4152d85f7077a62f184d19a750f2c06 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 11:29:05 +0000 Subject: [PATCH 03/18] test: add tests for DatabaseKeyRepository --- .../key/repository/persisted.rs | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/packages/tracker-core/src/authentication/key/repository/persisted.rs b/packages/tracker-core/src/authentication/key/repository/persisted.rs index 322ab291..b3948ca4 100644 --- a/packages/tracker-core/src/authentication/key/repository/persisted.rs +++ b/packages/tracker-core/src/authentication/key/repository/persisted.rs @@ -46,3 +46,76 @@ impl DatabaseKeyRepository { Ok(keys) } } + +#[cfg(test)] +mod tests { + + mod the_persisted_key_repository_should { + + use std::time::Duration; + + use torrust_tracker_test_helpers::configuration; + + use crate::authentication::key::repository::persisted::DatabaseKeyRepository; + use crate::authentication::{Key, PeerKey}; + use crate::databases::setup::initialize_database; + + #[test] + fn persist_a_new_peer_key() { + let configuration = configuration::ephemeral_public(); + + let database = initialize_database(&configuration); + + let repository = DatabaseKeyRepository::new(&database); + + let peer_key = PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: Some(Duration::new(9999, 0)), + }; + + let result = repository.add(&peer_key); + + assert!(result.is_ok()); + } + + #[test] + fn remove_a_persisted_peer_key() { + let configuration = configuration::ephemeral_public(); + + let database = initialize_database(&configuration); + + let repository = DatabaseKeyRepository::new(&database); + + let peer_key = PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: Some(Duration::new(9999, 0)), + }; + + let _unused = repository.add(&peer_key); + + let result = repository.remove(&peer_key.key); + + assert!(result.is_ok()); + } + + #[test] + fn load_all_persisted_peer_keys() { + let configuration = configuration::ephemeral_public(); + + let database = initialize_database(&configuration); + + let repository = DatabaseKeyRepository::new(&database); + + let peer_key = PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: Some(Duration::new(9999, 0)), + }; + + let _unused = repository.add(&peer_key); + + let keys = repository.load_keys().unwrap(); + + assert_eq!(keys, vec!(peer_key)); + } + } +} From f485a525ccd0e717eef0ae005061079d16092b71 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 11:51:46 +0000 Subject: [PATCH 04/18] refactor: inject only core config in tracker core DB setup --- packages/tracker-core/src/announce_handler.rs | 2 +- .../tracker-core/src/authentication/handler.rs | 2 +- .../authentication/key/repository/persisted.rs | 6 +++--- packages/tracker-core/src/authentication/mod.rs | 2 +- packages/tracker-core/src/core_tests.rs | 2 +- packages/tracker-core/src/databases/setup.rs | 15 ++++++--------- .../tracker-core/src/whitelist/whitelist_tests.rs | 2 +- src/bootstrap/app.rs | 2 +- src/servers/http/v1/handlers/announce.rs | 2 +- src/servers/http/v1/services/announce.rs | 4 ++-- src/servers/http/v1/services/scrape.rs | 2 +- src/servers/udp/handlers.rs | 4 ++-- 12 files changed, 21 insertions(+), 24 deletions(-) diff --git a/packages/tracker-core/src/announce_handler.rs b/packages/tracker-core/src/announce_handler.rs index 877555d1..fac1df5b 100644 --- a/packages/tracker-core/src/announce_handler.rs +++ b/packages/tracker-core/src/announce_handler.rs @@ -425,7 +425,7 @@ mod tests { config.core.tracker_policy.persistent_torrent_completed_stat = true; - let database = initialize_database(&config); + let database = initialize_database(&config.core); let in_memory_torrent_repository = Arc::new(InMemoryTorrentRepository::default()); let db_torrent_repository = Arc::new(DatabasePersistentTorrentRepository::new(&database)); let torrents_manager = Arc::new(TorrentsManager::new( diff --git a/packages/tracker-core/src/authentication/handler.rs b/packages/tracker-core/src/authentication/handler.rs index 1d74c7df..10ba3ecb 100644 --- a/packages/tracker-core/src/authentication/handler.rs +++ b/packages/tracker-core/src/authentication/handler.rs @@ -266,7 +266,7 @@ mod tests { } fn instantiate_keys_handler_with_configuration(config: &Configuration) -> KeysHandler { - let database = initialize_database(config); + let database = initialize_database(&config.core); let db_key_repository = Arc::new(DatabaseKeyRepository::new(&database)); let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); diff --git a/packages/tracker-core/src/authentication/key/repository/persisted.rs b/packages/tracker-core/src/authentication/key/repository/persisted.rs index b3948ca4..60db056a 100644 --- a/packages/tracker-core/src/authentication/key/repository/persisted.rs +++ b/packages/tracker-core/src/authentication/key/repository/persisted.rs @@ -64,7 +64,7 @@ mod tests { fn persist_a_new_peer_key() { let configuration = configuration::ephemeral_public(); - let database = initialize_database(&configuration); + let database = initialize_database(&configuration.core); let repository = DatabaseKeyRepository::new(&database); @@ -82,7 +82,7 @@ mod tests { fn remove_a_persisted_peer_key() { let configuration = configuration::ephemeral_public(); - let database = initialize_database(&configuration); + let database = initialize_database(&configuration.core); let repository = DatabaseKeyRepository::new(&database); @@ -102,7 +102,7 @@ mod tests { fn load_all_persisted_peer_keys() { let configuration = configuration::ephemeral_public(); - let database = initialize_database(&configuration); + let database = initialize_database(&configuration.core); let repository = DatabaseKeyRepository::new(&database); diff --git a/packages/tracker-core/src/authentication/mod.rs b/packages/tracker-core/src/authentication/mod.rs index 9609733d..4197f432 100644 --- a/packages/tracker-core/src/authentication/mod.rs +++ b/packages/tracker-core/src/authentication/mod.rs @@ -49,7 +49,7 @@ mod tests { fn instantiate_keys_manager_and_authentication_with_configuration( config: &Configuration, ) -> (Arc, Arc) { - let database = initialize_database(config); + let database = initialize_database(&config.core); let db_key_repository = Arc::new(DatabaseKeyRepository::new(&database)); let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); let authentication_service = Arc::new(service::AuthenticationService::new(&config.core, &in_memory_key_repository)); diff --git a/packages/tracker-core/src/core_tests.rs b/packages/tracker-core/src/core_tests.rs index 35d5fb9b..f6b47acd 100644 --- a/packages/tracker-core/src/core_tests.rs +++ b/packages/tracker-core/src/core_tests.rs @@ -84,7 +84,7 @@ pub fn incomplete_peer() -> Peer { #[must_use] pub fn initialize_handlers(config: &Configuration) -> (Arc, Arc) { - let database = initialize_database(config); + let database = initialize_database(&config.core); let in_memory_whitelist = Arc::new(InMemoryWhitelist::default()); let whitelist_authorization = Arc::new(whitelist::authorization::WhitelistAuthorization::new( &config.core, diff --git a/packages/tracker-core/src/databases/setup.rs b/packages/tracker-core/src/databases/setup.rs index 8d24c63b..73ff23fe 100644 --- a/packages/tracker-core/src/databases/setup.rs +++ b/packages/tracker-core/src/databases/setup.rs @@ -1,7 +1,6 @@ use std::sync::Arc; -use torrust_tracker_configuration::v2_0_0::database; -use torrust_tracker_configuration::Configuration; +use torrust_tracker_configuration::Core; use super::driver::{self, Driver}; use super::Database; @@ -10,13 +9,11 @@ use super::Database; /// /// Will panic if database cannot be initialized. #[must_use] -pub fn initialize_database(config: &Configuration) -> Arc> { - // todo: inject only core configuration - - let driver = match config.core.database.driver { - database::Driver::Sqlite3 => Driver::Sqlite3, - database::Driver::MySQL => Driver::MySQL, +pub fn initialize_database(config: &Core) -> Arc> { + let driver = match config.database.driver { + torrust_tracker_configuration::Driver::Sqlite3 => Driver::Sqlite3, + torrust_tracker_configuration::Driver::MySQL => Driver::MySQL, }; - Arc::new(driver::build(&driver, &config.core.database.path).expect("Database driver build failed.")) + Arc::new(driver::build(&driver, &config.database.path).expect("Database driver build failed.")) } diff --git a/packages/tracker-core/src/whitelist/whitelist_tests.rs b/packages/tracker-core/src/whitelist/whitelist_tests.rs index 33f5a97f..d2fd275f 100644 --- a/packages/tracker-core/src/whitelist/whitelist_tests.rs +++ b/packages/tracker-core/src/whitelist/whitelist_tests.rs @@ -10,7 +10,7 @@ use crate::whitelist::setup::initialize_whitelist_manager; #[must_use] pub fn initialize_whitelist_services(config: &Configuration) -> (Arc, Arc) { - let database = initialize_database(config); + let database = initialize_database(&config.core); let in_memory_whitelist = Arc::new(InMemoryWhitelist::default()); let whitelist_authorization = Arc::new(WhitelistAuthorization::new(&config.core, &in_memory_whitelist.clone())); let whitelist_manager = initialize_whitelist_manager(database.clone(), in_memory_whitelist.clone()); diff --git a/src/bootstrap/app.rs b/src/bootstrap/app.rs index 93bbfe29..e0e81c70 100644 --- a/src/bootstrap/app.rs +++ b/src/bootstrap/app.rs @@ -104,7 +104,7 @@ pub fn initialize_app_container(configuration: &Configuration) -> AppContainer { let udp_stats_repository = Arc::new(udp_stats_repository); let ban_service = Arc::new(RwLock::new(BanService::new(MAX_CONNECTION_ID_ERRORS_PER_IP))); - let database = initialize_database(configuration); + let database = initialize_database(&configuration.core); let in_memory_whitelist = Arc::new(InMemoryWhitelist::default()); let whitelist_authorization = Arc::new(WhitelistAuthorization::new(&configuration.core, &in_memory_whitelist.clone())); let whitelist_manager = initialize_whitelist_manager(database.clone(), in_memory_whitelist.clone()); diff --git a/src/servers/http/v1/handlers/announce.rs b/src/servers/http/v1/handlers/announce.rs index a6671e14..4c4aa661 100644 --- a/src/servers/http/v1/handlers/announce.rs +++ b/src/servers/http/v1/handlers/announce.rs @@ -294,7 +294,7 @@ mod tests { fn initialize_core_tracker_services(config: &Configuration) -> (CoreTrackerServices, CoreHttpTrackerServices) { let core_config = Arc::new(config.core.clone()); - let database = initialize_database(config); + let database = initialize_database(&config.core); let in_memory_whitelist = Arc::new(InMemoryWhitelist::default()); let whitelist_authorization = Arc::new(WhitelistAuthorization::new(&config.core, &in_memory_whitelist.clone())); let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); diff --git a/src/servers/http/v1/services/announce.rs b/src/servers/http/v1/services/announce.rs index bc21657a..64a29db5 100644 --- a/src/servers/http/v1/services/announce.rs +++ b/src/servers/http/v1/services/announce.rs @@ -84,7 +84,7 @@ mod tests { let config = configuration::ephemeral_public(); let core_config = Arc::new(config.core.clone()); - let database = initialize_database(&config); + let database = initialize_database(&config.core); let in_memory_torrent_repository = Arc::new(InMemoryTorrentRepository::default()); let db_torrent_repository = Arc::new(DatabasePersistentTorrentRepository::new(&database)); @@ -173,7 +173,7 @@ mod tests { fn initialize_announce_handler() -> Arc { let config = configuration::ephemeral(); - let database = initialize_database(&config); + let database = initialize_database(&config.core); let in_memory_torrent_repository = Arc::new(InMemoryTorrentRepository::default()); let db_torrent_repository = Arc::new(DatabasePersistentTorrentRepository::new(&database)); diff --git a/src/servers/http/v1/services/scrape.rs b/src/servers/http/v1/services/scrape.rs index 5325b188..0a3425ef 100644 --- a/src/servers/http/v1/services/scrape.rs +++ b/src/servers/http/v1/services/scrape.rs @@ -102,7 +102,7 @@ mod tests { fn initialize_announce_and_scrape_handlers_for_public_tracker() -> (Arc, Arc) { let config = configuration::ephemeral_public(); - let database = initialize_database(&config); + let database = initialize_database(&config.core); let in_memory_whitelist = Arc::new(InMemoryWhitelist::default()); let whitelist_authorization = Arc::new(WhitelistAuthorization::new(&config.core, &in_memory_whitelist.clone())); let in_memory_torrent_repository = Arc::new(InMemoryTorrentRepository::default()); diff --git a/src/servers/udp/handlers.rs b/src/servers/udp/handlers.rs index 59833b71..4f98f52d 100644 --- a/src/servers/udp/handlers.rs +++ b/src/servers/udp/handlers.rs @@ -536,7 +536,7 @@ mod tests { fn initialize_core_tracker_services(config: &Configuration) -> (CoreTrackerServices, CoreUdpTrackerServices) { let core_config = Arc::new(config.core.clone()); - let database = initialize_database(config); + let database = initialize_database(&config.core); let in_memory_whitelist = Arc::new(InMemoryWhitelist::default()); let whitelist_authorization = Arc::new(WhitelistAuthorization::new(&config.core, &in_memory_whitelist.clone())); let in_memory_torrent_repository = Arc::new(InMemoryTorrentRepository::default()); @@ -1470,7 +1470,7 @@ mod tests { async fn the_peer_ip_should_be_changed_to_the_external_ip_in_the_tracker_configuration() { let config = Arc::new(TrackerConfigurationBuilder::default().with_external_ip("::126.0.0.1").into()); - let database = initialize_database(&config); + let database = initialize_database(&config.core); let in_memory_whitelist = Arc::new(InMemoryWhitelist::default()); let whitelist_authorization = Arc::new(WhitelistAuthorization::new(&config.core, &in_memory_whitelist.clone())); From e519e7f826c1aee64cc4c284b33c3f3f4a1c4843 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 12:14:45 +0000 Subject: [PATCH 05/18] refactor: [#1231] simplify tests for DatabaseKeyRepository We don't need the whole tracker config. --- packages/test-helpers/src/configuration.rs | 13 +++++++---- .../key/repository/persisted.rs | 22 +++++++++++++------ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/packages/test-helpers/src/configuration.rs b/packages/test-helpers/src/configuration.rs index 678f4283..13082033 100644 --- a/packages/test-helpers/src/configuration.rs +++ b/packages/test-helpers/src/configuration.rs @@ -1,6 +1,7 @@ //! Tracker configuration factories for testing. use std::env; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; +use std::path::PathBuf; use std::time::Duration; use torrust_tracker_configuration::{Configuration, HttpApi, HttpTracker, Threshold, UdpTracker}; @@ -63,15 +64,19 @@ pub fn ephemeral() -> Configuration { tsl_config: None, }]); - // Ephemeral sqlite database - let temp_directory = env::temp_dir(); - let random_db_id = random::string(16); - let temp_file = temp_directory.join(format!("data_{random_db_id}.db")); + let temp_file = ephemeral_sqlite_database(); temp_file.to_str().unwrap().clone_into(&mut config.core.database.path); config } +#[must_use] +pub fn ephemeral_sqlite_database() -> PathBuf { + let temp_directory = env::temp_dir(); + let random_db_id = random::string(16); + temp_directory.join(format!("data_{random_db_id}.db")) +} + /// Ephemeral configuration with reverse proxy enabled. #[must_use] pub fn ephemeral_with_reverse_proxy() -> Configuration { diff --git a/packages/tracker-core/src/authentication/key/repository/persisted.rs b/packages/tracker-core/src/authentication/key/repository/persisted.rs index 60db056a..65e56cec 100644 --- a/packages/tracker-core/src/authentication/key/repository/persisted.rs +++ b/packages/tracker-core/src/authentication/key/repository/persisted.rs @@ -54,17 +54,25 @@ mod tests { use std::time::Duration; - use torrust_tracker_test_helpers::configuration; + use torrust_tracker_configuration::Core; + use torrust_tracker_test_helpers::configuration::ephemeral_sqlite_database; use crate::authentication::key::repository::persisted::DatabaseKeyRepository; use crate::authentication::{Key, PeerKey}; use crate::databases::setup::initialize_database; + fn ephemeral_configuration() -> Core { + let mut config = Core::default(); + let temp_file = ephemeral_sqlite_database(); + temp_file.to_str().unwrap().clone_into(&mut config.database.path); + config + } + #[test] fn persist_a_new_peer_key() { - let configuration = configuration::ephemeral_public(); + let configuration = ephemeral_configuration(); - let database = initialize_database(&configuration.core); + let database = initialize_database(&configuration); let repository = DatabaseKeyRepository::new(&database); @@ -80,9 +88,9 @@ mod tests { #[test] fn remove_a_persisted_peer_key() { - let configuration = configuration::ephemeral_public(); + let configuration = ephemeral_configuration(); - let database = initialize_database(&configuration.core); + let database = initialize_database(&configuration); let repository = DatabaseKeyRepository::new(&database); @@ -100,9 +108,9 @@ mod tests { #[test] fn load_all_persisted_peer_keys() { - let configuration = configuration::ephemeral_public(); + let configuration = ephemeral_configuration(); - let database = initialize_database(&configuration.core); + let database = initialize_database(&configuration); let repository = DatabaseKeyRepository::new(&database); From 87095400d7f91cd65c0c232e1f5bccd12af3a4a6 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 12:18:31 +0000 Subject: [PATCH 06/18] refactor: [#1231] improve DatabaseKeyRepository tests --- .../src/authentication/key/repository/persisted.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/tracker-core/src/authentication/key/repository/persisted.rs b/packages/tracker-core/src/authentication/key/repository/persisted.rs index 65e56cec..7edee62c 100644 --- a/packages/tracker-core/src/authentication/key/repository/persisted.rs +++ b/packages/tracker-core/src/authentication/key/repository/persisted.rs @@ -82,8 +82,10 @@ mod tests { }; let result = repository.add(&peer_key); - assert!(result.is_ok()); + + let keys = repository.load_keys().unwrap(); + assert_eq!(keys, vec!(peer_key)); } #[test] @@ -102,8 +104,10 @@ mod tests { let _unused = repository.add(&peer_key); let result = repository.remove(&peer_key.key); - assert!(result.is_ok()); + + let keys = repository.load_keys().unwrap(); + assert!(keys.is_empty()); } #[test] From 0336ca7a5d404d71c4604002c8535cf6f0cba2c8 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 12:31:45 +0000 Subject: [PATCH 07/18] refactor: [#1231] exctract mod --- .../src/authentication/key/mod.rs | 155 +------------ .../src/authentication/key/peer_key.rs | 206 ++++++++++++++++++ 2 files changed, 212 insertions(+), 149 deletions(-) create mode 100644 packages/tracker-core/src/authentication/key/peer_key.rs diff --git a/packages/tracker-core/src/authentication/key/mod.rs b/packages/tracker-core/src/authentication/key/mod.rs index e3e7fc01..081e027b 100644 --- a/packages/tracker-core/src/authentication/key/mod.rs +++ b/packages/tracker-core/src/authentication/key/mod.rs @@ -37,25 +37,26 @@ //! //! assert!(authentication::key::verify_key_expiration(&expiring_key).is_ok()); //! ``` +pub mod peer_key; pub mod repository; use std::panic::Location; -use std::str::FromStr; use std::sync::Arc; use std::time::Duration; -use derive_more::Display; use rand::distr::Alphanumeric; use rand::{rng, Rng}; -use serde::{Deserialize, Serialize}; use thiserror::Error; use torrust_tracker_clock::clock::Time; -use torrust_tracker_clock::conv::convert_from_timestamp_to_datetime_utc; use torrust_tracker_located_error::{DynError, LocatedError}; use torrust_tracker_primitives::DurationSinceUnixEpoch; use crate::CurrentClock; +pub type PeerKey = peer_key::PeerKey; +pub type Key = peer_key::Key; +pub type ParseKeyError = peer_key::ParseKeyError; + /// HTTP tracker authentication key length. /// /// For more information see function [`generate_key`](crate::authentication::key::generate_key) to generate the @@ -130,110 +131,6 @@ pub fn verify_key_expiration(auth_key: &PeerKey) -> Result<(), Error> { } } -/// An authentication key which can potentially have an expiration time. -/// After that time is will automatically become invalid. -#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)] -pub struct PeerKey { - /// Random 32-char string. For example: `YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ` - pub key: Key, - - /// Timestamp, the key will be no longer valid after this timestamp. - /// If `None` the keys will not expire (permanent key). - pub valid_until: Option, -} - -impl std::fmt::Display for PeerKey { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self.expiry_time() { - Some(expire_time) => write!(f, "key: `{}`, valid until `{}`", self.key, expire_time), - None => write!(f, "key: `{}`, permanent", self.key), - } - } -} - -impl PeerKey { - #[must_use] - pub fn key(&self) -> Key { - self.key.clone() - } - - /// It returns the expiry time. For example, for the starting time for Unix Epoch - /// (timestamp 0) it will return a `DateTime` whose string representation is - /// `1970-01-01 00:00:00 UTC`. - /// - /// # Panics - /// - /// Will panic when the key timestamp overflows the internal i64 type. - /// (this will naturally happen in 292.5 billion years) - #[must_use] - pub fn expiry_time(&self) -> Option> { - self.valid_until.map(convert_from_timestamp_to_datetime_utc) - } -} - -/// A token used for authentication. -/// -/// - It contains only ascii alphanumeric chars: lower and uppercase letters and -/// numbers. -/// - It's a 32-char string. -#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone, Display, Hash)] -pub struct Key(String); - -impl Key { - /// # Errors - /// - /// Will return an error is the string represents an invalid key. - /// Valid keys can only contain 32 chars including 0-9, a-z and A-Z. - pub fn new(value: &str) -> Result { - if value.len() != AUTH_KEY_LENGTH { - return Err(ParseKeyError::InvalidKeyLength); - } - - if !value.chars().all(|c| c.is_ascii_alphanumeric()) { - return Err(ParseKeyError::InvalidChars); - } - - Ok(Self(value.to_owned())) - } - - #[must_use] - pub fn value(&self) -> &str { - &self.0 - } -} - -/// Error returned when a key cannot be parsed from a string. -/// -/// ```text -/// use bittorrent_tracker_core::authentication::Key; -/// use std::str::FromStr; -/// -/// let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; -/// let key = Key::from_str(key_string); -/// -/// assert!(key.is_ok()); -/// assert_eq!(key.unwrap().to_string(), key_string); -/// ``` -/// -/// If the string does not contains a valid key, the parser function will return -/// this error. -#[derive(Debug, Error)] -pub enum ParseKeyError { - #[error("Invalid key length. Key must be have 32 chars")] - InvalidKeyLength, - #[error("Invalid chars for key. Key can only alphanumeric chars (0-9, a-z, A-Z)")] - InvalidChars, -} - -impl FromStr for Key { - type Err = ParseKeyError; - - fn from_str(s: &str) -> Result { - Key::new(s)?; - Ok(Self(s.to_string())) - } -} - /// Verification error. Error returned when an [`PeerKey`] cannot be /// verified with the (`crate::authentication::verify_key`) function. #[derive(Debug, Error)] @@ -263,39 +160,8 @@ impl From for Error { #[cfg(test)] mod tests { - mod key { - use std::str::FromStr; - - use crate::authentication::Key; - - #[test] - fn should_be_parsed_from_an_string() { - let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; - let key = Key::from_str(key_string); - - assert!(key.is_ok()); - assert_eq!(key.unwrap().to_string(), key_string); - } - - #[test] - fn length_should_be_32() { - let key = Key::new(""); - assert!(key.is_err()); - - let string_longer_than_32 = "012345678901234567890123456789012"; // DevSkim: ignore DS173237 - let key = Key::new(string_longer_than_32); - assert!(key.is_err()); - } - - #[test] - fn should_only_include_alphanumeric_chars() { - let key = Key::new("%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%"); - assert!(key.is_err()); - } - } - mod expiring_auth_key { - use std::str::FromStr; + use std::time::Duration; use torrust_tracker_clock::clock; @@ -303,15 +169,6 @@ mod tests { use crate::authentication; - #[test] - fn should_be_parsed_from_an_string() { - let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; - let auth_key = authentication::Key::from_str(key_string); - - assert!(auth_key.is_ok()); - assert_eq!(auth_key.unwrap().to_string(), key_string); - } - #[test] fn should_be_displayed() { // Set the time to the current time. diff --git a/packages/tracker-core/src/authentication/key/peer_key.rs b/packages/tracker-core/src/authentication/key/peer_key.rs new file mode 100644 index 00000000..c4dfc774 --- /dev/null +++ b/packages/tracker-core/src/authentication/key/peer_key.rs @@ -0,0 +1,206 @@ +use std::str::FromStr; + +use derive_more::Display; +use serde::{Deserialize, Serialize}; +use thiserror::Error; +use torrust_tracker_clock::conv::convert_from_timestamp_to_datetime_utc; +use torrust_tracker_primitives::DurationSinceUnixEpoch; + +use super::AUTH_KEY_LENGTH; + +/// An authentication key which can potentially have an expiration time. +/// After that time is will automatically become invalid. +#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)] +pub struct PeerKey { + /// Random 32-char string. For example: `YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ` + pub key: Key, + + /// Timestamp, the key will be no longer valid after this timestamp. + /// If `None` the keys will not expire (permanent key). + pub valid_until: Option, +} + +impl std::fmt::Display for PeerKey { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self.expiry_time() { + Some(expire_time) => write!(f, "key: `{}`, valid until `{}`", self.key, expire_time), + None => write!(f, "key: `{}`, permanent", self.key), + } + } +} + +impl PeerKey { + #[must_use] + pub fn key(&self) -> Key { + self.key.clone() + } + + /// It returns the expiry time. For example, for the starting time for Unix Epoch + /// (timestamp 0) it will return a `DateTime` whose string representation is + /// `1970-01-01 00:00:00 UTC`. + /// + /// # Panics + /// + /// Will panic when the key timestamp overflows the internal i64 type. + /// (this will naturally happen in 292.5 billion years) + #[must_use] + pub fn expiry_time(&self) -> Option> { + self.valid_until.map(convert_from_timestamp_to_datetime_utc) + } +} + +/// A token used for authentication. +/// +/// - It contains only ascii alphanumeric chars: lower and uppercase letters and +/// numbers. +/// - It's a 32-char string. +#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone, Display, Hash)] +pub struct Key(String); + +impl Key { + /// # Errors + /// + /// Will return an error is the string represents an invalid key. + /// Valid keys can only contain 32 chars including 0-9, a-z and A-Z. + pub fn new(value: &str) -> Result { + if value.len() != AUTH_KEY_LENGTH { + return Err(ParseKeyError::InvalidKeyLength); + } + + if !value.chars().all(|c| c.is_ascii_alphanumeric()) { + return Err(ParseKeyError::InvalidChars); + } + + Ok(Self(value.to_owned())) + } + + #[must_use] + pub fn value(&self) -> &str { + &self.0 + } +} + +/// Error returned when a key cannot be parsed from a string. +/// +/// ```text +/// use bittorrent_tracker_core::authentication::Key; +/// use std::str::FromStr; +/// +/// let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; +/// let key = Key::from_str(key_string); +/// +/// assert!(key.is_ok()); +/// assert_eq!(key.unwrap().to_string(), key_string); +/// ``` +/// +/// If the string does not contains a valid key, the parser function will return +/// this error. +#[derive(Debug, Error)] +pub enum ParseKeyError { + #[error("Invalid key length. Key must be have 32 chars")] + InvalidKeyLength, + #[error("Invalid chars for key. Key can only alphanumeric chars (0-9, a-z, A-Z)")] + InvalidChars, +} + +impl FromStr for Key { + type Err = ParseKeyError; + + fn from_str(s: &str) -> Result { + Key::new(s)?; + Ok(Self(s.to_string())) + } +} + +#[cfg(test)] +mod tests { + + mod key { + use std::str::FromStr; + + use crate::authentication::Key; + + #[test] + fn should_be_parsed_from_an_string() { + let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; + let key = Key::from_str(key_string); + + assert!(key.is_ok()); + assert_eq!(key.unwrap().to_string(), key_string); + } + + #[test] + fn length_should_be_32() { + let key = Key::new(""); + assert!(key.is_err()); + + let string_longer_than_32 = "012345678901234567890123456789012"; // DevSkim: ignore DS173237 + let key = Key::new(string_longer_than_32); + assert!(key.is_err()); + } + + #[test] + fn should_only_include_alphanumeric_chars() { + let key = Key::new("%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%"); + assert!(key.is_err()); + } + } + + mod expiring_auth_key { + use std::str::FromStr; + use std::time::Duration; + + use torrust_tracker_clock::clock; + use torrust_tracker_clock::clock::stopped::Stopped as _; + + use crate::authentication; + + #[test] + fn should_be_parsed_from_an_string() { + let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; + let auth_key = authentication::Key::from_str(key_string); + + assert!(auth_key.is_ok()); + assert_eq!(auth_key.unwrap().to_string(), key_string); + } + + #[test] + fn should_be_displayed() { + // Set the time to the current time. + clock::Stopped::local_set_to_unix_epoch(); + + let expiring_key = authentication::key::generate_key(Some(Duration::from_secs(0))); + + assert_eq!( + expiring_key.to_string(), + format!("key: `{}`, valid until `1970-01-01 00:00:00 UTC`", expiring_key.key) // cspell:disable-line + ); + } + + #[test] + fn should_be_generated_with_a_expiration_time() { + let expiring_key = authentication::key::generate_key(Some(Duration::new(9999, 0))); + + assert!(authentication::key::verify_key_expiration(&expiring_key).is_ok()); + } + + #[test] + fn should_be_generate_and_verified() { + // Set the time to the current time. + clock::Stopped::local_set_to_system_time_now(); + + // Make key that is valid for 19 seconds. + let expiring_key = authentication::key::generate_key(Some(Duration::from_secs(19))); + + // Mock the time has passed 10 sec. + clock::Stopped::local_add(&Duration::from_secs(10)).unwrap(); + + assert!(authentication::key::verify_key_expiration(&expiring_key).is_ok()); + + // Mock the time has passed another 10 sec. + clock::Stopped::local_add(&Duration::from_secs(10)).unwrap(); + + assert!(authentication::key::verify_key_expiration(&expiring_key).is_err()); + } + } +} From 5d91a326734cfc1f35cfa6bb845cdeae82a93a49 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 12:59:50 +0000 Subject: [PATCH 08/18] test: [#1231] add more tests to peer_key mod --- .../src/authentication/key/peer_key.rs | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/tracker-core/src/authentication/key/peer_key.rs b/packages/tracker-core/src/authentication/key/peer_key.rs index c4dfc774..9b330185 100644 --- a/packages/tracker-core/src/authentication/key/peer_key.rs +++ b/packages/tracker-core/src/authentication/key/peer_key.rs @@ -99,6 +99,7 @@ impl Key { pub enum ParseKeyError { #[error("Invalid key length. Key must be have 32 chars")] InvalidKeyLength, + #[error("Invalid chars for key. Key can only alphanumeric chars (0-9, a-z, A-Z)")] InvalidChars, } @@ -144,9 +145,16 @@ mod tests { let key = Key::new("%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%"); assert!(key.is_err()); } + + #[test] + fn should_return_a_reference_to_the_inner_string() { + let key = Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); // DevSkim: ignore DS173237 + + assert_eq!(key.value(), "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"); // DevSkim: ignore DS173237 + } } - mod expiring_auth_key { + mod peer_key { use std::str::FromStr; use std::time::Duration; @@ -165,7 +173,7 @@ mod tests { } #[test] - fn should_be_displayed() { + fn should_be_displayed_when_it_is_expiring() { // Set the time to the current time. clock::Stopped::local_set_to_unix_epoch(); @@ -177,6 +185,16 @@ mod tests { ); } + #[test] + fn should_be_displayed_when_it_is_permanent() { + let expiring_key = authentication::key::generate_permanent_key(); + + assert_eq!( + expiring_key.to_string(), + format!("key: `{}`, permanent", expiring_key.key) // cspell:disable-line + ); + } + #[test] fn should_be_generated_with_a_expiration_time() { let expiring_key = authentication::key::generate_key(Some(Duration::new(9999, 0))); From d0c7313056d0baa9f7883c2bb90cea7e00e5a419 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 13:23:35 +0000 Subject: [PATCH 09/18] refactor: [#1231] peer_key mod tests --- .../src/authentication/key/mod.rs | 16 +-- .../src/authentication/key/peer_key.rs | 107 ++++++++++-------- 2 files changed, 67 insertions(+), 56 deletions(-) diff --git a/packages/tracker-core/src/authentication/key/mod.rs b/packages/tracker-core/src/authentication/key/mod.rs index 081e027b..228e4c68 100644 --- a/packages/tracker-core/src/authentication/key/mod.rs +++ b/packages/tracker-core/src/authentication/key/mod.rs @@ -44,8 +44,6 @@ use std::panic::Location; use std::sync::Arc; use std::time::Duration; -use rand::distr::Alphanumeric; -use rand::{rng, Rng}; use thiserror::Error; use torrust_tracker_clock::clock::Time; use torrust_tracker_located_error::{DynError, LocatedError}; @@ -82,24 +80,20 @@ pub fn generate_permanent_key() -> PeerKey { /// * `lifetime`: if `None` the key will be permanent. #[must_use] pub fn generate_key(lifetime: Option) -> PeerKey { - let random_id: String = rng() - .sample_iter(&Alphanumeric) - .take(AUTH_KEY_LENGTH) - .map(char::from) - .collect(); + let random_key = Key::random(); if let Some(lifetime) = lifetime { - tracing::debug!("Generated key: {}, valid for: {:?} seconds", random_id, lifetime); + tracing::debug!("Generated key: {}, valid for: {:?} seconds", random_key, lifetime); PeerKey { - key: random_id.parse::().unwrap(), + key: random_key, valid_until: Some(CurrentClock::now_add(&lifetime).unwrap()), } } else { - tracing::debug!("Generated key: {}, permanent", random_id); + tracing::debug!("Generated key: {}, permanent", random_key); PeerKey { - key: random_id.parse::().unwrap(), + key: random_key, valid_until: None, } } diff --git a/packages/tracker-core/src/authentication/key/peer_key.rs b/packages/tracker-core/src/authentication/key/peer_key.rs index 9b330185..a3045e54 100644 --- a/packages/tracker-core/src/authentication/key/peer_key.rs +++ b/packages/tracker-core/src/authentication/key/peer_key.rs @@ -1,6 +1,8 @@ use std::str::FromStr; use derive_more::Display; +use rand::distr::Alphanumeric; +use rand::{rng, Rng}; use serde::{Deserialize, Serialize}; use thiserror::Error; use torrust_tracker_clock::conv::convert_from_timestamp_to_datetime_utc; @@ -74,6 +76,20 @@ impl Key { Ok(Self(value.to_owned())) } + /// It generates a random key. + /// + /// # Panics + /// + /// Will panic if the random number generator fails to generate a valid key. + pub fn random() -> Self { + let random_id: String = rng() + .sample_iter(&Alphanumeric) + .take(AUTH_KEY_LENGTH) + .map(char::from) + .collect(); + random_id.parse::().expect("Failed to generate a valid random key") + } + #[must_use] pub fn value(&self) -> &str { &self.0 @@ -130,6 +146,11 @@ mod tests { assert_eq!(key.unwrap().to_string(), key_string); } + #[test] + fn should_be_generated_randomly() { + let _key = Key::random(); + } + #[test] fn length_should_be_32() { let key = Key::new(""); @@ -155,70 +176,66 @@ mod tests { } mod peer_key { - use std::str::FromStr; - use std::time::Duration; - use torrust_tracker_clock::clock; - use torrust_tracker_clock::clock::stopped::Stopped as _; + use std::time::Duration; - use crate::authentication; + use crate::authentication::key::peer_key::{Key, PeerKey}; #[test] - fn should_be_parsed_from_an_string() { - let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; - let auth_key = authentication::Key::from_str(key_string); + fn could_have_an_expiration_time() { + let expiring_key = PeerKey { + key: Key::random(), + valid_until: Some(Duration::from_secs(100)), + }; - assert!(auth_key.is_ok()); - assert_eq!(auth_key.unwrap().to_string(), key_string); + assert_eq!(expiring_key.expiry_time().unwrap().to_string(), "1970-01-01 00:01:40 UTC"); } #[test] - fn should_be_displayed_when_it_is_expiring() { - // Set the time to the current time. - clock::Stopped::local_set_to_unix_epoch(); + fn could_be_permanent() { + let permanent_key = PeerKey { + key: Key::random(), + valid_until: None, + }; - let expiring_key = authentication::key::generate_key(Some(Duration::from_secs(0))); - - assert_eq!( - expiring_key.to_string(), - format!("key: `{}`, valid until `1970-01-01 00:00:00 UTC`", expiring_key.key) // cspell:disable-line - ); + assert_eq!(permanent_key.expiry_time(), None); } - #[test] - fn should_be_displayed_when_it_is_permanent() { - let expiring_key = authentication::key::generate_permanent_key(); + mod expiring { + use std::time::Duration; - assert_eq!( - expiring_key.to_string(), - format!("key: `{}`, permanent", expiring_key.key) // cspell:disable-line - ); - } + use crate::authentication::key::peer_key::{Key, PeerKey}; - #[test] - fn should_be_generated_with_a_expiration_time() { - let expiring_key = authentication::key::generate_key(Some(Duration::new(9999, 0))); + #[test] + fn should_be_displayed_when_it_is_expiring() { + let expiring_key = PeerKey { + key: Key::random(), + valid_until: Some(Duration::from_secs(100)), + }; - assert!(authentication::key::verify_key_expiration(&expiring_key).is_ok()); + assert_eq!( + expiring_key.to_string(), + format!("key: `{}`, valid until `1970-01-01 00:01:40 UTC`", expiring_key.key) // cspell:disable-line + ); + } } - #[test] - fn should_be_generate_and_verified() { - // Set the time to the current time. - clock::Stopped::local_set_to_system_time_now(); - - // Make key that is valid for 19 seconds. - let expiring_key = authentication::key::generate_key(Some(Duration::from_secs(19))); - - // Mock the time has passed 10 sec. - clock::Stopped::local_add(&Duration::from_secs(10)).unwrap(); + mod permanent { - assert!(authentication::key::verify_key_expiration(&expiring_key).is_ok()); + use crate::authentication::key::peer_key::{Key, PeerKey}; - // Mock the time has passed another 10 sec. - clock::Stopped::local_add(&Duration::from_secs(10)).unwrap(); + #[test] + fn should_be_displayed_when_it_is_permanent() { + let permanent_key = PeerKey { + key: Key::random(), + valid_until: None, + }; - assert!(authentication::key::verify_key_expiration(&expiring_key).is_err()); + assert_eq!( + permanent_key.to_string(), + format!("key: `{}`, permanent", permanent_key.key) // cspell:disable-line + ); + } } } } From 0d7e30e579ca61991113fd5ab96dd57a387aad3e Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 16:01:21 +0000 Subject: [PATCH 10/18] refactor: [#1231] bittorrent_tracker_core::authentication::key::tests --- .../src/authentication/key/mod.rs | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/packages/tracker-core/src/authentication/key/mod.rs b/packages/tracker-core/src/authentication/key/mod.rs index 228e4c68..bdb72b1c 100644 --- a/packages/tracker-core/src/authentication/key/mod.rs +++ b/packages/tracker-core/src/authentication/key/mod.rs @@ -134,11 +134,13 @@ pub enum Error { KeyVerificationError { source: LocatedError<'static, dyn std::error::Error + Send + Sync>, }, + #[error("Failed to read key: {key}, {location}")] UnableToReadKey { location: &'static Location<'static>, key: Box, }, + #[error("Key has expired, {location}")] KeyExpired { location: &'static Location<'static> }, } @@ -154,7 +156,7 @@ impl From for Error { #[cfg(test)] mod tests { - mod expiring_auth_key { + mod the_expiring_peer_key { use std::time::Duration; @@ -184,7 +186,7 @@ mod tests { } #[test] - fn should_be_generate_and_verified() { + fn expiration_verification_should_fail_when_the_key_has_expired() { // Set the time to the current time. clock::Stopped::local_set_to_system_time_now(); @@ -202,4 +204,44 @@ mod tests { assert!(authentication::key::verify_key_expiration(&expiring_key).is_err()); } } + + mod the_permanent_peer_key { + + use std::time::Duration; + + use torrust_tracker_clock::clock; + use torrust_tracker_clock::clock::stopped::Stopped as _; + + use crate::authentication; + + #[test] + fn should_be_displayed() { + // Set the time to the current time. + clock::Stopped::local_set_to_unix_epoch(); + + let expiring_key = authentication::key::generate_key(Some(Duration::from_secs(0))); + + assert_eq!( + expiring_key.to_string(), + format!("key: `{}`, valid until `1970-01-01 00:00:00 UTC`", expiring_key.key) // cspell:disable-line + ); + } + + #[test] + fn should_be_generated_without_expiration_time() { + let expiring_key = authentication::key::generate_permanent_key(); + + assert!(authentication::key::verify_key_expiration(&expiring_key).is_ok()); + } + + #[test] + fn expiration_verification_should_always_succeed() { + let expiring_key = authentication::key::generate_permanent_key(); + + // Mock the time has passed 10 years. + clock::Stopped::local_add(&Duration::from_secs(10 * 365 * 24 * 60 * 60)).unwrap(); + + assert!(authentication::key::verify_key_expiration(&expiring_key).is_ok()); + } + } } From 5db73be398c80d10fee73f39734c3513b5e09be9 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 16:12:01 +0000 Subject: [PATCH 11/18] refactor: rename methods --- .../src/authentication/handler.rs | 36 ++++++++++--------- .../tracker-core/src/authentication/mod.rs | 23 ++++++++---- src/app.rs | 2 +- .../apis/v1/context/auth_key/handlers.rs | 9 +++-- .../api/v1/contract/context/auth_key.rs | 14 ++++---- tests/servers/http/v1/contract.rs | 4 +-- 6 files changed, 52 insertions(+), 36 deletions(-) diff --git a/packages/tracker-core/src/authentication/handler.rs b/packages/tracker-core/src/authentication/handler.rs index 10ba3ecb..4c392ee5 100644 --- a/packages/tracker-core/src/authentication/handler.rs +++ b/packages/tracker-core/src/authentication/handler.rs @@ -54,8 +54,6 @@ impl KeysHandler { /// - The provided pre-generated key is invalid. /// - The key could not been persisted due to database issues. pub async fn add_peer_key(&self, add_key_req: AddKeyRequest) -> Result { - // code-review: all methods related to keys should be moved to a new independent "keys" service. - match add_key_req.opt_key { // Upload pre-generated key Some(pre_existing_key) => { @@ -68,7 +66,7 @@ impl KeysHandler { let key = pre_existing_key.parse::(); match key { - Ok(key) => match self.add_auth_key(key, Some(valid_until)).await { + Ok(key) => match self.add_expiring_peer_key(key, Some(valid_until)).await { Ok(auth_key) => Ok(auth_key), Err(err) => Err(PeerKeyError::DatabaseError { source: Located(err).into(), @@ -84,7 +82,7 @@ impl KeysHandler { let key = pre_existing_key.parse::(); match key { - Ok(key) => match self.add_permanent_auth_key(key).await { + Ok(key) => match self.add_permanent_peer_key(key).await { Ok(auth_key) => Ok(auth_key), Err(err) => Err(PeerKeyError::DatabaseError { source: Located(err).into(), @@ -100,14 +98,17 @@ impl KeysHandler { // Generate a new random key None => match add_key_req.opt_seconds_valid { // Expiring key - Some(seconds_valid) => match self.generate_auth_key(Some(Duration::from_secs(seconds_valid))).await { + Some(seconds_valid) => match self + .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) + .await + { Ok(auth_key) => Ok(auth_key), Err(err) => Err(PeerKeyError::DatabaseError { source: Located(err).into(), }), }, // Permanent key - None => match self.generate_permanent_auth_key().await { + None => match self.generate_permanent_peer_key().await { Ok(auth_key) => Ok(auth_key), Err(err) => Err(PeerKeyError::DatabaseError { source: Located(err).into(), @@ -124,8 +125,8 @@ impl KeysHandler { /// # Errors /// /// Will return a `database::Error` if unable to add the `auth_key` to the database. - pub async fn generate_permanent_auth_key(&self) -> Result { - self.generate_auth_key(None).await + pub async fn generate_permanent_peer_key(&self) -> Result { + self.generate_expiring_peer_key(None).await } /// It generates a new expiring authentication key. @@ -140,7 +141,7 @@ impl KeysHandler { /// /// * `lifetime` - The duration in seconds for the new key. The key will be /// no longer valid after `lifetime` seconds. - pub async fn generate_auth_key(&self, lifetime: Option) -> Result { + pub async fn generate_expiring_peer_key(&self, lifetime: Option) -> Result { let peer_key = key::generate_key(lifetime); self.db_key_repository.add(&peer_key)?; @@ -162,8 +163,8 @@ impl KeysHandler { /// # Arguments /// /// * `key` - The pre-generated key. - pub async fn add_permanent_auth_key(&self, key: Key) -> Result { - self.add_auth_key(key, None).await + pub async fn add_permanent_peer_key(&self, key: Key) -> Result { + self.add_expiring_peer_key(key, None).await } /// It adds a pre-generated authentication key. @@ -180,7 +181,7 @@ impl KeysHandler { /// * `key` - The pre-generated key. /// * `lifetime` - The duration in seconds for the new key. The key will be /// no longer valid after `lifetime` seconds. - pub async fn add_auth_key( + pub async fn add_expiring_peer_key( &self, key: Key, valid_until: Option, @@ -202,7 +203,7 @@ impl KeysHandler { /// # Errors /// /// Will return a `database::Error` if unable to remove the `key` to the database. - pub async fn remove_auth_key(&self, key: &Key) -> Result<(), databases::error::Error> { + pub async fn remove_peer_key(&self, key: &Key) -> Result<(), databases::error::Error> { self.db_key_repository.remove(key)?; self.remove_in_memory_auth_key(key).await; @@ -223,7 +224,7 @@ impl KeysHandler { /// # Errors /// /// Will return a `database::Error` if unable to `load_keys` from the database. - pub async fn load_keys_from_database(&self) -> Result<(), databases::error::Error> { + pub async fn load_peer_keys_from_database(&self) -> Result<(), databases::error::Error> { let keys_from_database = self.db_key_repository.load_keys()?; self.in_memory_key_repository.reset_with(keys_from_database).await; @@ -287,7 +288,10 @@ mod tests { async fn it_should_generate_the_key() { let keys_handler = instantiate_keys_handler(); - let peer_key = keys_handler.generate_auth_key(Some(Duration::from_secs(100))).await.unwrap(); + let peer_key = keys_handler + .generate_expiring_peer_key(Some(Duration::from_secs(100))) + .await + .unwrap(); assert_eq!( peer_key.valid_until, @@ -335,7 +339,7 @@ mod tests { async fn it_should_generate_the_key() { let keys_handler = instantiate_keys_handler(); - let peer_key = keys_handler.generate_permanent_auth_key().await.unwrap(); + let peer_key = keys_handler.generate_permanent_peer_key().await.unwrap(); assert_eq!(peer_key.valid_until, None); } diff --git a/packages/tracker-core/src/authentication/mod.rs b/packages/tracker-core/src/authentication/mod.rs index 4197f432..52138d26 100644 --- a/packages/tracker-core/src/authentication/mod.rs +++ b/packages/tracker-core/src/authentication/mod.rs @@ -65,9 +65,12 @@ mod tests { async fn it_should_remove_an_authentication_key() { let (keys_manager, authentication_service) = instantiate_keys_manager_and_authentication(); - let expiring_key = keys_manager.generate_auth_key(Some(Duration::from_secs(100))).await.unwrap(); + let expiring_key = keys_manager + .generate_expiring_peer_key(Some(Duration::from_secs(100))) + .await + .unwrap(); - let result = keys_manager.remove_auth_key(&expiring_key.key()).await; + let result = keys_manager.remove_peer_key(&expiring_key.key()).await; assert!(result.is_ok()); @@ -79,12 +82,15 @@ mod tests { async fn it_should_load_authentication_keys_from_the_database() { let (keys_manager, authentication_service) = instantiate_keys_manager_and_authentication(); - let expiring_key = keys_manager.generate_auth_key(Some(Duration::from_secs(100))).await.unwrap(); + let expiring_key = keys_manager + .generate_expiring_peer_key(Some(Duration::from_secs(100))) + .await + .unwrap(); // Remove the newly generated key in memory keys_manager.remove_in_memory_auth_key(&expiring_key.key()).await; - let result = keys_manager.load_keys_from_database().await; + let result = keys_manager.load_peer_keys_from_database().await; assert!(result.is_ok()); @@ -107,7 +113,10 @@ mod tests { async fn it_should_authenticate_a_peer_with_the_key() { let (keys_manager, authentication_service) = instantiate_keys_manager_and_authentication(); - let peer_key = keys_manager.generate_auth_key(Some(Duration::from_secs(100))).await.unwrap(); + let peer_key = keys_manager + .generate_expiring_peer_key(Some(Duration::from_secs(100))) + .await + .unwrap(); let result = authentication_service.authenticate(&peer_key.key()).await; @@ -122,7 +131,7 @@ mod tests { let past_timestamp = Duration::ZERO; let peer_key = keys_manager - .add_auth_key(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), Some(past_timestamp)) + .add_expiring_peer_key(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), Some(past_timestamp)) .await .unwrap(); @@ -183,7 +192,7 @@ mod tests { async fn it_should_authenticate_a_peer_with_the_key() { let (keys_manager, authentication_service) = instantiate_keys_manager_and_authentication(); - let peer_key = keys_manager.generate_permanent_auth_key().await.unwrap(); + let peer_key = keys_manager.generate_permanent_peer_key().await.unwrap(); let result = authentication_service.authenticate(&peer_key.key()).await; diff --git a/src/app.rs b/src/app.rs index d69874eb..ad752437 100644 --- a/src/app.rs +++ b/src/app.rs @@ -55,7 +55,7 @@ pub async fn start(config: &Configuration, app_container: &Arc) -> if config.core.private { app_container .keys_handler - .load_keys_from_database() + .load_peer_keys_from_database() .await .expect("Could not retrieve keys from database."); } diff --git a/src/servers/apis/v1/context/auth_key/handlers.rs b/src/servers/apis/v1/context/auth_key/handlers.rs index ca38ade3..c8d4c25b 100644 --- a/src/servers/apis/v1/context/auth_key/handlers.rs +++ b/src/servers/apis/v1/context/auth_key/handlers.rs @@ -70,7 +70,10 @@ pub async fn generate_auth_key_handler( Path(seconds_valid_or_key): Path, ) -> Response { let seconds_valid = seconds_valid_or_key; - match keys_handler.generate_auth_key(Some(Duration::from_secs(seconds_valid))).await { + match keys_handler + .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) + .await + { Ok(auth_key) => auth_key_response(&AuthKey::from(auth_key)), Err(e) => failed_to_generate_key_response(e), } @@ -111,7 +114,7 @@ pub async fn delete_auth_key_handler( ) -> Response { match Key::from_str(&seconds_valid_or_key.0) { Err(_) => invalid_auth_key_param_response(&seconds_valid_or_key.0), - Ok(key) => match keys_handler.remove_auth_key(&key).await { + Ok(key) => match keys_handler.remove_peer_key(&key).await { Ok(()) => ok_response(), Err(e) => failed_to_delete_key_response(e), }, @@ -131,7 +134,7 @@ pub async fn delete_auth_key_handler( /// Refer to the [API endpoint documentation](crate::servers::apis::v1::context::auth_key#reload-authentication-keys) /// for more information about this endpoint. pub async fn reload_keys_handler(State(keys_handler): State>) -> Response { - match keys_handler.load_keys_from_database().await { + match keys_handler.load_peer_keys_from_database().await { Ok(()) => ok_response(), Err(e) => failed_to_reload_keys_response(e), } diff --git a/tests/servers/api/v1/contract/context/auth_key.rs b/tests/servers/api/v1/contract/context/auth_key.rs index 47cf0ecd..ab9bfaf3 100644 --- a/tests/servers/api/v1/contract/context/auth_key.rs +++ b/tests/servers/api/v1/contract/context/auth_key.rs @@ -160,7 +160,7 @@ async fn should_allow_deleting_an_auth_key() { let auth_key = env .http_api_container .keys_handler - .generate_auth_key(Some(Duration::from_secs(seconds_valid))) + .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) .await .unwrap(); @@ -295,7 +295,7 @@ async fn should_fail_when_the_auth_key_cannot_be_deleted() { let auth_key = env .http_api_container .keys_handler - .generate_auth_key(Some(Duration::from_secs(seconds_valid))) + .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) .await .unwrap(); @@ -329,7 +329,7 @@ async fn should_not_allow_deleting_an_auth_key_for_unauthenticated_users() { let auth_key = env .http_api_container .keys_handler - .generate_auth_key(Some(Duration::from_secs(seconds_valid))) + .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) .await .unwrap(); @@ -350,7 +350,7 @@ async fn should_not_allow_deleting_an_auth_key_for_unauthenticated_users() { let auth_key = env .http_api_container .keys_handler - .generate_auth_key(Some(Duration::from_secs(seconds_valid))) + .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) .await .unwrap(); @@ -379,7 +379,7 @@ async fn should_allow_reloading_keys() { let seconds_valid = 60; env.http_api_container .keys_handler - .generate_auth_key(Some(Duration::from_secs(seconds_valid))) + .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) .await .unwrap(); @@ -405,7 +405,7 @@ async fn should_fail_when_keys_cannot_be_reloaded() { env.http_api_container .keys_handler - .generate_auth_key(Some(Duration::from_secs(seconds_valid))) + .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) .await .unwrap(); @@ -434,7 +434,7 @@ async fn should_not_allow_reloading_keys_for_unauthenticated_users() { let seconds_valid = 60; env.http_api_container .keys_handler - .generate_auth_key(Some(Duration::from_secs(seconds_valid))) + .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) .await .unwrap(); diff --git a/tests/servers/http/v1/contract.rs b/tests/servers/http/v1/contract.rs index 48c98fa0..bab96940 100644 --- a/tests/servers/http/v1/contract.rs +++ b/tests/servers/http/v1/contract.rs @@ -1404,7 +1404,7 @@ mod configured_as_private { let expiring_key = env .keys_handler - .generate_auth_key(Some(Duration::from_secs(60))) + .generate_expiring_peer_key(Some(Duration::from_secs(60))) .await .unwrap(); @@ -1553,7 +1553,7 @@ mod configured_as_private { let expiring_key = env .keys_handler - .generate_auth_key(Some(Duration::from_secs(60))) + .generate_expiring_peer_key(Some(Duration::from_secs(60))) .await .unwrap(); From e3ba1e198701e3d8cf94d274f4d6bf85b628c551 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 3 Feb 2025 17:50:16 +0000 Subject: [PATCH 12/18] test: [#1231] add tests for KeysHandler --- .../src/authentication/handler.rs | 200 +++++++++++++++--- 1 file changed, 176 insertions(+), 24 deletions(-) diff --git a/packages/tracker-core/src/authentication/handler.rs b/packages/tracker-core/src/authentication/handler.rs index 4c392ee5..3643e7ec 100644 --- a/packages/tracker-core/src/authentication/handler.rs +++ b/packages/tracker-core/src/authentication/handler.rs @@ -44,7 +44,8 @@ impl KeysHandler { /// Adds new peer keys to the tracker. /// - /// Keys can be pre-generated or randomly created. They can also be permanent or expire. + /// Keys can be pre-generated or randomly created. They can also be + /// permanent or expire. /// /// # Errors /// @@ -55,8 +56,9 @@ impl KeysHandler { /// - The key could not been persisted due to database issues. pub async fn add_peer_key(&self, add_key_req: AddKeyRequest) -> Result { match add_key_req.opt_key { - // Upload pre-generated key Some(pre_existing_key) => { + // Upload pre-generated key + if let Some(seconds_valid) = add_key_req.opt_seconds_valid { // Expiring key let Some(valid_until) = CurrentClock::now_add(&Duration::from_secs(seconds_valid)) else { @@ -95,20 +97,20 @@ impl KeysHandler { } } } - // Generate a new random key None => match add_key_req.opt_seconds_valid { - // Expiring key + // Generate a new random key Some(seconds_valid) => match self .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) .await { + // Expiring key Ok(auth_key) => Ok(auth_key), Err(err) => Err(PeerKeyError::DatabaseError { source: Located(err).into(), }), }, - // Permanent key None => match self.generate_permanent_peer_key().await { + // Permanent key Ok(auth_key) => Ok(auth_key), Err(err) => Err(PeerKeyError::DatabaseError { source: Located(err).into(), @@ -236,7 +238,7 @@ impl KeysHandler { #[cfg(test)] mod tests { - mod the_keys_handler_when_tracker_is_configured_as_private { + mod the_keys_handler_when_the_tracker_is_configured_as_private { use std::sync::Arc; @@ -267,6 +269,8 @@ mod tests { } fn instantiate_keys_handler_with_configuration(config: &Configuration) -> KeysHandler { + // todo: pass only Core configuration + let database = initialize_database(&config.core); let db_key_repository = Arc::new(DatabaseKeyRepository::new(&database)); let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); @@ -274,22 +278,48 @@ mod tests { KeysHandler::new(&db_key_repository, &in_memory_key_repository) } - mod with_expiring_and { + mod handling_expiring_peer_keys { - mod randomly_generated_keys { + use std::time::Duration; + + use torrust_tracker_clock::clock::Time; + + use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; + use crate::CurrentClock; + + #[tokio::test] + async fn it_should_generate_the_key() { + let keys_handler = instantiate_keys_handler(); + + let peer_key = keys_handler + .generate_expiring_peer_key(Some(Duration::from_secs(100))) + .await + .unwrap(); + + assert_eq!( + peer_key.valid_until, + Some(CurrentClock::now_add(&Duration::from_secs(100)).unwrap()) + ); + } + + mod randomly_generated { use std::time::Duration; use torrust_tracker_clock::clock::Time; - use crate::authentication::handler::tests::the_keys_handler_when_tracker_is_configured_as_private::instantiate_keys_handler; + use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; + use crate::authentication::handler::AddKeyRequest; use crate::CurrentClock; #[tokio::test] - async fn it_should_generate_the_key() { + async fn it_should_add_a_randomly_generated_key() { let keys_handler = instantiate_keys_handler(); let peer_key = keys_handler - .generate_expiring_peer_key(Some(Duration::from_secs(100))) + .add_peer_key(AddKeyRequest { + opt_key: None, + opt_seconds_valid: Some(100), + }) .await .unwrap(); @@ -300,14 +330,20 @@ mod tests { } } - mod pre_generated_keys { + mod pre_generated { + use std::sync::Arc; use std::time::Duration; use torrust_tracker_clock::clock::Time; - - use crate::authentication::handler::tests::the_keys_handler_when_tracker_is_configured_as_private::instantiate_keys_handler; - use crate::authentication::handler::AddKeyRequest; - use crate::authentication::Key; + use torrust_tracker_test_helpers::configuration; + + use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; + use crate::authentication::handler::{AddKeyRequest, KeysHandler}; + use crate::authentication::key::repository::in_memory::InMemoryKeyRepository; + use crate::authentication::key::repository::persisted::DatabaseKeyRepository; + use crate::authentication::{Key, PeerKey}; + use crate::databases::setup::initialize_database; + use crate::error::PeerKeyError; use crate::CurrentClock; #[tokio::test] @@ -323,17 +359,59 @@ mod tests { .unwrap(); assert_eq!( - peer_key.valid_until, - Some(CurrentClock::now_add(&Duration::from_secs(100)).unwrap()) + peer_key, + PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: Some(CurrentClock::now_add(&Duration::from_secs(100)).unwrap()), + } ); } + + #[tokio::test] + async fn it_should_fail_adding_a_pre_generated_key_when_the_key_is_invalid() { + let keys_handler = instantiate_keys_handler(); + + let result = keys_handler + .add_peer_key(AddKeyRequest { + opt_key: Some("INVALID KEY".to_string()), + opt_seconds_valid: Some(100), + }) + .await; + + assert!(matches!(result.unwrap_err(), PeerKeyError::InvalidKey { .. })); + } + + #[tokio::test] + async fn it_should_fail_adding_a_pre_generated_key_when_there_is_a_database_error() { + let config = configuration::ephemeral_private(); + let database = initialize_database(&config.core); + let db_key_repository = Arc::new(DatabaseKeyRepository::new(&database)); + let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); + + // Force database error. + // todo: extract trait for DatabaseKeyRepository to be able + // to mock it. Test should be faster if we don't have to + // create a new database. + let _unused = database.drop_database_tables(); + + let keys_handler = KeysHandler::new(&db_key_repository, &in_memory_key_repository); + + let result = keys_handler + .add_peer_key(AddKeyRequest { + opt_key: Some(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap().to_string()), + opt_seconds_valid: Some(100), + }) + .await; + + assert!(matches!(result.unwrap_err(), PeerKeyError::DatabaseError { .. })); + } } } - mod with_permanent_and { + mod handling_permanent_peer_keys { mod randomly_generated_keys { - use crate::authentication::handler::tests::the_keys_handler_when_tracker_is_configured_as_private::instantiate_keys_handler; + use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; #[tokio::test] async fn it_should_generate_the_key() { @@ -345,11 +423,40 @@ mod tests { } } - mod pre_generated_keys { + mod randomly_generated { - use crate::authentication::handler::tests::the_keys_handler_when_tracker_is_configured_as_private::instantiate_keys_handler; + use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; use crate::authentication::handler::AddKeyRequest; - use crate::authentication::Key; + + #[tokio::test] + async fn it_should_add_a_randomly_generated_key() { + let keys_handler = instantiate_keys_handler(); + + let peer_key = keys_handler + .add_peer_key(AddKeyRequest { + opt_key: None, + opt_seconds_valid: None, + }) + .await + .unwrap(); + + assert_eq!(peer_key.valid_until, None); + } + } + + mod pre_generated_keys { + + use std::sync::Arc; + + use torrust_tracker_test_helpers::configuration; + + use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; + use crate::authentication::handler::{AddKeyRequest, KeysHandler}; + use crate::authentication::key::repository::in_memory::InMemoryKeyRepository; + use crate::authentication::key::repository::persisted::DatabaseKeyRepository; + use crate::authentication::{Key, PeerKey}; + use crate::databases::setup::initialize_database; + use crate::error::PeerKeyError; #[tokio::test] async fn it_should_add_a_pre_generated_key() { @@ -363,7 +470,52 @@ mod tests { .await .unwrap(); - assert_eq!(peer_key.valid_until, None); + assert_eq!( + peer_key, + PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: None, + } + ); + } + + #[tokio::test] + async fn it_should_fail_adding_a_pre_generated_key_when_the_key_is_invalid() { + let keys_handler = instantiate_keys_handler(); + + let result = keys_handler + .add_peer_key(AddKeyRequest { + opt_key: Some("INVALID KEY".to_string()), + opt_seconds_valid: None, + }) + .await; + + assert!(matches!(result.unwrap_err(), PeerKeyError::InvalidKey { .. })); + } + + #[tokio::test] + async fn it_should_fail_adding_a_pre_generated_key_when_there_is_a_database_error() { + let config = configuration::ephemeral_private(); + let database = initialize_database(&config.core); + let db_key_repository = Arc::new(DatabaseKeyRepository::new(&database)); + let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); + + // Force database error. + // todo: extract trait for DatabaseKeyRepository to be able + // to mock it. Test should be faster if we don't have to + // create a new database. + let _unused = database.drop_database_tables(); + + let keys_handler = KeysHandler::new(&db_key_repository, &in_memory_key_repository); + + let result = keys_handler + .add_peer_key(AddKeyRequest { + opt_key: Some("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".to_string()), + opt_seconds_valid: None, + }) + .await; + + assert!(matches!(result.unwrap_err(), PeerKeyError::DatabaseError { .. })); } } } From bd4cef66842aaab3f23c811362ad56ceb311d4fe Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 4 Feb 2025 08:56:31 +0000 Subject: [PATCH 13/18] refactor: [#1231] tests to use database mock in KeysHandler tests that require the `Database` trait. --- packages/tracker-core/Cargo.toml | 1 + .../src/authentication/handler.rs | 294 ++++++++++++------ packages/tracker-core/src/databases/mod.rs | 2 + 3 files changed, 207 insertions(+), 90 deletions(-) diff --git a/packages/tracker-core/Cargo.toml b/packages/tracker-core/Cargo.toml index 7b5b1f2c..aeea30a3 100644 --- a/packages/tracker-core/Cargo.toml +++ b/packages/tracker-core/Cargo.toml @@ -20,6 +20,7 @@ bittorrent-http-protocol = { version = "3.0.0-develop", path = "../http-protocol bittorrent-primitives = "0.1.0" chrono = { version = "0", default-features = false, features = ["clock"] } derive_more = { version = "1", features = ["as_ref", "constructor", "from"] } +mockall = "0" r2d2 = "0" r2d2_mysql = "25" r2d2_sqlite = { version = "0", features = ["bundled"] } diff --git a/packages/tracker-core/src/authentication/handler.rs b/packages/tracker-core/src/authentication/handler.rs index 3643e7ec..f733f5cb 100644 --- a/packages/tracker-core/src/authentication/handler.rs +++ b/packages/tracker-core/src/authentication/handler.rs @@ -55,68 +55,73 @@ impl KeysHandler { /// - The provided pre-generated key is invalid. /// - The key could not been persisted due to database issues. pub async fn add_peer_key(&self, add_key_req: AddKeyRequest) -> Result { - match add_key_req.opt_key { - Some(pre_existing_key) => { - // Upload pre-generated key - - if let Some(seconds_valid) = add_key_req.opt_seconds_valid { - // Expiring key - let Some(valid_until) = CurrentClock::now_add(&Duration::from_secs(seconds_valid)) else { - return Err(PeerKeyError::DurationOverflow { seconds_valid }); - }; + if let Some(pre_existing_key) = add_key_req.opt_key { + // Pre-generated key + + if let Some(seconds_valid) = add_key_req.opt_seconds_valid { + // Expiring key + + let Some(valid_until) = CurrentClock::now_add(&Duration::from_secs(seconds_valid)) else { + return Err(PeerKeyError::DurationOverflow { seconds_valid }); + }; - let key = pre_existing_key.parse::(); - - match key { - Ok(key) => match self.add_expiring_peer_key(key, Some(valid_until)).await { - Ok(auth_key) => Ok(auth_key), - Err(err) => Err(PeerKeyError::DatabaseError { - source: Located(err).into(), - }), - }, - Err(err) => Err(PeerKeyError::InvalidKey { - key: pre_existing_key, + let key = pre_existing_key.parse::(); + + match key { + Ok(key) => match self.add_expiring_peer_key(key, Some(valid_until)).await { + Ok(auth_key) => Ok(auth_key), + Err(err) => Err(PeerKeyError::DatabaseError { source: Located(err).into(), }), - } - } else { - // Permanent key - let key = pre_existing_key.parse::(); - - match key { - Ok(key) => match self.add_permanent_peer_key(key).await { - Ok(auth_key) => Ok(auth_key), - Err(err) => Err(PeerKeyError::DatabaseError { - source: Located(err).into(), - }), - }, - Err(err) => Err(PeerKeyError::InvalidKey { - key: pre_existing_key, + }, + Err(err) => Err(PeerKeyError::InvalidKey { + key: pre_existing_key, + source: Located(err).into(), + }), + } + } else { + // Permanent key + + let key = pre_existing_key.parse::(); + + match key { + Ok(key) => match self.add_permanent_peer_key(key).await { + Ok(auth_key) => Ok(auth_key), + Err(err) => Err(PeerKeyError::DatabaseError { source: Located(err).into(), }), - } + }, + Err(err) => Err(PeerKeyError::InvalidKey { + key: pre_existing_key, + source: Located(err).into(), + }), } } - None => match add_key_req.opt_seconds_valid { - // Generate a new random key - Some(seconds_valid) => match self + } else { + // New randomly generate key + + if let Some(seconds_valid) = add_key_req.opt_seconds_valid { + // Expiring key + + match self .generate_expiring_peer_key(Some(Duration::from_secs(seconds_valid))) .await { - // Expiring key Ok(auth_key) => Ok(auth_key), Err(err) => Err(PeerKeyError::DatabaseError { source: Located(err).into(), }), - }, - None => match self.generate_permanent_peer_key().await { - // Permanent key + } + } else { + // Permanent key + + match self.generate_permanent_peer_key().await { Ok(auth_key) => Ok(auth_key), Err(err) => Err(PeerKeyError::DatabaseError { source: Located(err).into(), }), - }, - }, + } + } } } @@ -250,6 +255,7 @@ mod tests { use crate::authentication::key::repository::in_memory::InMemoryKeyRepository; use crate::authentication::key::repository::persisted::DatabaseKeyRepository; use crate::databases::setup::initialize_database; + use crate::databases::Database; fn instantiate_keys_handler() -> KeysHandler { let config = configuration::ephemeral_private(); @@ -268,6 +274,13 @@ mod tests { instantiate_keys_handler_with_configuration(&config) } + fn instantiate_keys_handler_with_database(database: &Arc>) -> KeysHandler { + let db_key_repository = Arc::new(DatabaseKeyRepository::new(database)); + let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); + + KeysHandler::new(&db_key_repository, &in_memory_key_repository) + } + fn instantiate_keys_handler_with_configuration(config: &Configuration) -> KeysHandler { // todo: pass only Core configuration @@ -303,12 +316,22 @@ mod tests { } mod randomly_generated { + use std::panic::Location; + use std::sync::Arc; use std::time::Duration; - use torrust_tracker_clock::clock::Time; + use mockall::predicate::function; + use torrust_tracker_clock::clock::stopped::Stopped; + use torrust_tracker_clock::clock::{self, Time}; - use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; + use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::{ + instantiate_keys_handler, instantiate_keys_handler_with_database, + }; use crate::authentication::handler::AddKeyRequest; + use crate::authentication::PeerKey; + use crate::databases::driver::Driver; + use crate::databases::{self, Database, MockDatabase}; + use crate::error::PeerKeyError; use crate::CurrentClock; #[tokio::test] @@ -328,21 +351,58 @@ mod tests { Some(CurrentClock::now_add(&Duration::from_secs(100)).unwrap()) ); } + + #[tokio::test] + async fn it_should_fail_adding_a_randomly_generated_key_when_there_is_a_database_error() { + clock::Stopped::local_set(&Duration::from_secs(0)); + + // The key should be valid the next 60 seconds. + let expected_valid_until = clock::Stopped::now_add(&Duration::from_secs(60)).unwrap(); + + let mut database_mock = MockDatabase::default(); + database_mock + .expect_add_key_to_keys() + .with(function(move |peer_key: &PeerKey| { + peer_key.valid_until == Some(expected_valid_until) + })) + .times(1) + .returning(|_peer_key| { + Err(databases::error::Error::InsertFailed { + location: Location::caller(), + driver: Driver::Sqlite3, + }) + }); + let database_mock: Arc> = Arc::new(Box::new(database_mock)); + + let keys_handler = instantiate_keys_handler_with_database(&database_mock); + + let result = keys_handler + .add_peer_key(AddKeyRequest { + opt_key: None, + opt_seconds_valid: Some(60), // The key is valid for 60 seconds. + }) + .await; + + assert!(matches!(result.unwrap_err(), PeerKeyError::DatabaseError { .. })); + } } mod pre_generated { + use std::panic::Location; use std::sync::Arc; use std::time::Duration; - use torrust_tracker_clock::clock::Time; - use torrust_tracker_test_helpers::configuration; + use mockall::predicate; + use torrust_tracker_clock::clock::stopped::Stopped; + use torrust_tracker_clock::clock::{self, Time}; - use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; - use crate::authentication::handler::{AddKeyRequest, KeysHandler}; - use crate::authentication::key::repository::in_memory::InMemoryKeyRepository; - use crate::authentication::key::repository::persisted::DatabaseKeyRepository; + use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::{ + instantiate_keys_handler, instantiate_keys_handler_with_database, + }; + use crate::authentication::handler::AddKeyRequest; use crate::authentication::{Key, PeerKey}; - use crate::databases::setup::initialize_database; + use crate::databases::driver::Driver; + use crate::databases::{self, Database, MockDatabase}; use crate::error::PeerKeyError; use crate::CurrentClock; @@ -383,23 +443,34 @@ mod tests { #[tokio::test] async fn it_should_fail_adding_a_pre_generated_key_when_there_is_a_database_error() { - let config = configuration::ephemeral_private(); - let database = initialize_database(&config.core); - let db_key_repository = Arc::new(DatabaseKeyRepository::new(&database)); - let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); + clock::Stopped::local_set(&Duration::from_secs(0)); - // Force database error. - // todo: extract trait for DatabaseKeyRepository to be able - // to mock it. Test should be faster if we don't have to - // create a new database. - let _unused = database.drop_database_tables(); + // The key should be valid the next 60 seconds. + let expected_valid_until = clock::Stopped::now_add(&Duration::from_secs(60)).unwrap(); + let expected_peer_key = PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: Some(expected_valid_until), + }; - let keys_handler = KeysHandler::new(&db_key_repository, &in_memory_key_repository); + let mut database_mock = MockDatabase::default(); + database_mock + .expect_add_key_to_keys() + .with(predicate::eq(expected_peer_key)) + .times(1) + .returning(|_peer_key| { + Err(databases::error::Error::InsertFailed { + location: Location::caller(), + driver: Driver::Sqlite3, + }) + }); + let database_mock: Arc> = Arc::new(Box::new(database_mock)); + + let keys_handler = instantiate_keys_handler_with_database(&database_mock); let result = keys_handler .add_peer_key(AddKeyRequest { opt_key: Some(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap().to_string()), - opt_seconds_valid: Some(100), + opt_seconds_valid: Some(60), // The key is valid for 60 seconds. }) .await; @@ -410,8 +481,21 @@ mod tests { mod handling_permanent_peer_keys { - mod randomly_generated_keys { - use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; + mod randomly_generated { + + use std::panic::Location; + use std::sync::Arc; + + use mockall::predicate::function; + + use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::{ + instantiate_keys_handler, instantiate_keys_handler_with_database, + }; + use crate::authentication::handler::AddKeyRequest; + use crate::authentication::PeerKey; + use crate::databases::driver::Driver; + use crate::databases::{self, Database, MockDatabase}; + use crate::error::PeerKeyError; #[tokio::test] async fn it_should_generate_the_key() { @@ -421,12 +505,6 @@ mod tests { assert_eq!(peer_key.valid_until, None); } - } - - mod randomly_generated { - - use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; - use crate::authentication::handler::AddKeyRequest; #[tokio::test] async fn it_should_add_a_randomly_generated_key() { @@ -442,20 +520,49 @@ mod tests { assert_eq!(peer_key.valid_until, None); } + + #[tokio::test] + async fn it_should_fail_adding_a_randomly_generated_key_when_there_is_a_database_error() { + let mut database_mock = MockDatabase::default(); + database_mock + .expect_add_key_to_keys() + .with(function(move |peer_key: &PeerKey| peer_key.valid_until.is_none())) + .times(1) + .returning(|_peer_key| { + Err(databases::error::Error::InsertFailed { + location: Location::caller(), + driver: Driver::Sqlite3, + }) + }); + let database_mock: Arc> = Arc::new(Box::new(database_mock)); + + let keys_handler = instantiate_keys_handler_with_database(&database_mock); + + let result = keys_handler + .add_peer_key(AddKeyRequest { + opt_key: None, + opt_seconds_valid: None, + }) + .await; + + assert!(matches!(result.unwrap_err(), PeerKeyError::DatabaseError { .. })); + } } mod pre_generated_keys { + use std::panic::Location; use std::sync::Arc; - use torrust_tracker_test_helpers::configuration; + use mockall::predicate; - use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::instantiate_keys_handler; - use crate::authentication::handler::{AddKeyRequest, KeysHandler}; - use crate::authentication::key::repository::in_memory::InMemoryKeyRepository; - use crate::authentication::key::repository::persisted::DatabaseKeyRepository; + use crate::authentication::handler::tests::the_keys_handler_when_the_tracker_is_configured_as_private::{ + instantiate_keys_handler, instantiate_keys_handler_with_database, + }; + use crate::authentication::handler::AddKeyRequest; use crate::authentication::{Key, PeerKey}; - use crate::databases::setup::initialize_database; + use crate::databases::driver::Driver; + use crate::databases::{self, Database, MockDatabase}; use crate::error::PeerKeyError; #[tokio::test] @@ -495,22 +602,29 @@ mod tests { #[tokio::test] async fn it_should_fail_adding_a_pre_generated_key_when_there_is_a_database_error() { - let config = configuration::ephemeral_private(); - let database = initialize_database(&config.core); - let db_key_repository = Arc::new(DatabaseKeyRepository::new(&database)); - let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); - - // Force database error. - // todo: extract trait for DatabaseKeyRepository to be able - // to mock it. Test should be faster if we don't have to - // create a new database. - let _unused = database.drop_database_tables(); + let expected_peer_key = PeerKey { + key: Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), + valid_until: None, + }; - let keys_handler = KeysHandler::new(&db_key_repository, &in_memory_key_repository); + let mut database_mock = MockDatabase::default(); + database_mock + .expect_add_key_to_keys() + .with(predicate::eq(expected_peer_key)) + .times(1) + .returning(|_peer_key| { + Err(databases::error::Error::InsertFailed { + location: Location::caller(), + driver: Driver::Sqlite3, + }) + }); + let database_mock: Arc> = Arc::new(Box::new(database_mock)); + + let keys_handler = instantiate_keys_handler_with_database(&database_mock); let result = keys_handler .add_peer_key(AddKeyRequest { - opt_key: Some("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".to_string()), + opt_key: Some(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap().to_string()), opt_seconds_valid: None, }) .await; diff --git a/packages/tracker-core/src/databases/mod.rs b/packages/tracker-core/src/databases/mod.rs index 9b9ac8e9..f0930d05 100644 --- a/packages/tracker-core/src/databases/mod.rs +++ b/packages/tracker-core/src/databases/mod.rs @@ -52,6 +52,7 @@ pub mod sqlite; use std::marker::PhantomData; use bittorrent_primitives::info_hash::InfoHash; +use mockall::automock; use torrust_tracker_primitives::PersistentTorrents; use self::error::Error; @@ -79,6 +80,7 @@ where } /// The persistence trait. It contains all the methods to interact with the database. +#[automock] pub trait Database: Sync + Send { /// It instantiates a new database driver. /// From c3117cfb961610568ab2f558a3d55f63d4dce223 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 4 Feb 2025 12:17:13 +0000 Subject: [PATCH 14/18] test: [#1231] add more tests for KeysHandler --- .../tracker-core/src/authentication/handler.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/tracker-core/src/authentication/handler.rs b/packages/tracker-core/src/authentication/handler.rs index f733f5cb..3b708a51 100644 --- a/packages/tracker-core/src/authentication/handler.rs +++ b/packages/tracker-core/src/authentication/handler.rs @@ -427,6 +427,20 @@ mod tests { ); } + #[tokio::test] + async fn it_should_fail_adding_a_pre_generated_key_when_the_key_duration_exceeds_the_maximum_duration() { + let keys_handler = instantiate_keys_handler(); + + let result = keys_handler + .add_peer_key(AddKeyRequest { + opt_key: Some(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap().to_string()), + opt_seconds_valid: Some(u64::MAX), + }) + .await; + + assert!(matches!(result.unwrap_err(), PeerKeyError::DurationOverflow { .. })); + } + #[tokio::test] async fn it_should_fail_adding_a_pre_generated_key_when_the_key_is_invalid() { let keys_handler = instantiate_keys_handler(); From 63e773afca456ffba74683ec890f364974233d17 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 4 Feb 2025 12:23:54 +0000 Subject: [PATCH 15/18] refactor: [#1231] remove dead code --- packages/tracker-core/src/authentication/handler.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/packages/tracker-core/src/authentication/handler.rs b/packages/tracker-core/src/authentication/handler.rs index 3b708a51..f758830a 100644 --- a/packages/tracker-core/src/authentication/handler.rs +++ b/packages/tracker-core/src/authentication/handler.rs @@ -247,7 +247,6 @@ mod tests { use std::sync::Arc; - use torrust_tracker_configuration::v2_0_0::core::PrivateMode; use torrust_tracker_configuration::Configuration; use torrust_tracker_test_helpers::configuration; @@ -263,17 +262,6 @@ mod tests { instantiate_keys_handler_with_configuration(&config) } - #[allow(dead_code)] - fn instantiate_keys_handler_with_checking_keys_expiration_disabled() -> KeysHandler { - let mut config = configuration::ephemeral_private(); - - config.core.private_mode = Some(PrivateMode { - check_keys_expiration: false, - }); - - instantiate_keys_handler_with_configuration(&config) - } - fn instantiate_keys_handler_with_database(database: &Arc>) -> KeysHandler { let db_key_repository = Arc::new(DatabaseKeyRepository::new(database)); let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); From 7d8b3948c8a977935b01affa7f4d061dec853d93 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 4 Feb 2025 12:44:07 +0000 Subject: [PATCH 16/18] test: [#1231] add tests for AuthenticationService --- .../src/authentication/service.rs | 202 ++++++++++++++++-- 1 file changed, 183 insertions(+), 19 deletions(-) diff --git a/packages/tracker-core/src/authentication/service.rs b/packages/tracker-core/src/authentication/service.rs index 3e32bfbc..98b8a398 100644 --- a/packages/tracker-core/src/authentication/service.rs +++ b/packages/tracker-core/src/authentication/service.rs @@ -31,7 +31,7 @@ impl AuthenticationService { /// /// Will return an error if the the authentication key cannot be verified. pub async fn authenticate(&self, key: &Key) -> Result<(), Error> { - if self.is_private() { + if self.tracker_is_private() { self.verify_auth_key(key).await } else { Ok(()) @@ -40,7 +40,7 @@ impl AuthenticationService { /// Returns `true` is the tracker is in private mode. #[must_use] - pub fn is_private(&self) -> bool { + fn tracker_is_private(&self) -> bool { self.config.private } @@ -72,34 +72,198 @@ impl AuthenticationService { #[cfg(test)] mod tests { - mod the_tracker_configured_as_private { + mod the_authentication_service { - use std::str::FromStr; - use std::sync::Arc; + mod when_the_tracker_is_public { - use torrust_tracker_test_helpers::configuration; + use std::str::FromStr; + use std::sync::Arc; - use crate::authentication; - use crate::authentication::key::repository::in_memory::InMemoryKeyRepository; - use crate::authentication::service::AuthenticationService; + use torrust_tracker_configuration::Core; - fn instantiate_authentication() -> AuthenticationService { - let config = configuration::ephemeral_private(); + use crate::authentication::key::repository::in_memory::InMemoryKeyRepository; + use crate::authentication::service::AuthenticationService; + use crate::authentication::{self}; - let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); + fn instantiate_authentication_for_public_tracker() -> AuthenticationService { + let config = Core { + private: false, + ..Default::default() + }; - AuthenticationService::new(&config.core, &in_memory_key_repository.clone()) + let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); + + AuthenticationService::new(&config, &in_memory_key_repository.clone()) + } + + #[tokio::test] + async fn it_should_always_authenticate_when_the_tracker_is_public() { + let authentication = instantiate_authentication_for_public_tracker(); + + let unregistered_key = authentication::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + + let result = authentication.authenticate(&unregistered_key).await; + + assert!(result.is_ok()); + } } - #[tokio::test] - async fn it_should_not_authenticate_an_unregistered_key() { - let authentication = instantiate_authentication(); + mod when_the_tracker_is_private { + + use std::str::FromStr; + use std::sync::Arc; + use std::time::Duration; + + use torrust_tracker_configuration::v2_0_0::core::PrivateMode; + use torrust_tracker_configuration::Core; + + use crate::authentication::key::repository::in_memory::InMemoryKeyRepository; + use crate::authentication::service::AuthenticationService; + use crate::authentication::{self, PeerKey}; + + fn instantiate_authentication_for_private_tracker() -> AuthenticationService { + let config = Core { + private: true, + ..Default::default() + }; + + let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); + + AuthenticationService::new(&config, &in_memory_key_repository.clone()) + } + + #[tokio::test] + async fn it_should_authenticate_a_registered_key() { + let config = Core { + private: true, + ..Default::default() + }; + + let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); + + let key = authentication::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + + in_memory_key_repository + .insert(&PeerKey { + key: key.clone(), + valid_until: None, + }) + .await; + + let authentication = AuthenticationService::new(&config, &in_memory_key_repository.clone()); + + let result = authentication.authenticate(&key).await; + + assert!(result.is_ok()); + } + + #[tokio::test] + async fn it_should_not_authenticate_an_unregistered_key() { + let authentication = instantiate_authentication_for_private_tracker(); + + let unregistered_key = authentication::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + + let result = authentication.authenticate(&unregistered_key).await; + + assert!(result.is_err()); + } - let unregistered_key = authentication::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + #[tokio::test] + async fn it_should_not_authenticate_a_registered_but_expired_key_by_default() { + let config = Core { + private: true, + ..Default::default() + }; - let result = authentication.authenticate(&unregistered_key).await; + let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); - assert!(result.is_err()); + let key = authentication::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + + // Register the key with an immediate expiration date. + in_memory_key_repository + .insert(&PeerKey { + key: key.clone(), + valid_until: Some(Duration::from_secs(0)), + }) + .await; + + let authentication = AuthenticationService::new(&config, &in_memory_key_repository.clone()); + + let result = authentication.authenticate(&key).await; + + assert!(result.is_err()); + } + + #[tokio::test] + async fn it_should_not_authenticate_a_registered_but_expired_key_when_the_tracker_is_explicitly_configured_to_check_keys_expiration() { + let config = Core { + private: true, + private_mode: Some(PrivateMode { + check_keys_expiration: true, + }), + ..Default::default() + }; + + let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); + + let key = authentication::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + + // Register the key with an immediate expiration date. + in_memory_key_repository + .insert(&PeerKey { + key: key.clone(), + valid_until: Some(Duration::from_secs(0)), + }) + .await; + + let authentication = AuthenticationService::new(&config, &in_memory_key_repository.clone()); + + let result = authentication.authenticate(&key).await; + + assert!(result.is_err()); + } + + mod but_the_key_expiration_check_is_disabled_by_configuration { + use std::str::FromStr; + use std::sync::Arc; + use std::time::Duration; + + use torrust_tracker_configuration::v2_0_0::core::PrivateMode; + use torrust_tracker_configuration::Core; + + use crate::authentication::key::repository::in_memory::InMemoryKeyRepository; + use crate::authentication::service::AuthenticationService; + use crate::authentication::{self, PeerKey}; + + #[tokio::test] + async fn it_should_authenticate_an_expired_registered_key() { + let config = Core { + private: true, + private_mode: Some(PrivateMode { + check_keys_expiration: false, + }), + ..Default::default() + }; + + let in_memory_key_repository = Arc::new(InMemoryKeyRepository::default()); + + let key = authentication::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + + // Register the key with an immediate expiration date. + in_memory_key_repository + .insert(&PeerKey { + key: key.clone(), + valid_until: Some(Duration::from_secs(0)), + }) + .await; + + let authentication = AuthenticationService::new(&config, &in_memory_key_repository.clone()); + + let result = authentication.authenticate(&key).await; + + assert!(result.is_ok()); + } + } } } } From 3e02b48c676c2355547bf8292fbd5a4ac6c8a349 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 4 Feb 2025 13:18:05 +0000 Subject: [PATCH 17/18] test: [#1231] add more tests for authentication::service mod --- .../src/authentication/key/mod.rs | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/tracker-core/src/authentication/key/mod.rs b/packages/tracker-core/src/authentication/key/mod.rs index bdb72b1c..33b3b609 100644 --- a/packages/tracker-core/src/authentication/key/mod.rs +++ b/packages/tracker-core/src/authentication/key/mod.rs @@ -104,10 +104,8 @@ pub fn generate_key(lifetime: Option) -> PeerKey { /// /// # Errors /// -/// Will return: -/// -/// - `Error::KeyExpired` if `auth_key.valid_until` is past the `current_time`. -/// - `Error::KeyInvalid` if `auth_key.valid_until` is past the `None`. +/// Will return a verification error [`crate::authentication::key::Error`] if +/// it cannot verify the key. pub fn verify_key_expiration(auth_key: &PeerKey) -> Result<(), Error> { let current_time: DurationSinceUnixEpoch = CurrentClock::now(); @@ -126,7 +124,7 @@ pub fn verify_key_expiration(auth_key: &PeerKey) -> Result<(), Error> { } /// Verification error. Error returned when an [`PeerKey`] cannot be -/// verified with the (`crate::authentication::verify_key`) function. +/// verified with the [`crate::authentication::key::verify_key_expiration`] function. #[derive(Debug, Error)] #[allow(dead_code)] pub enum Error { @@ -244,4 +242,17 @@ mod tests { assert!(authentication::key::verify_key_expiration(&expiring_key).is_ok()); } } + + mod the_key_verification_error { + use crate::authentication::key; + + #[test] + fn could_be_a_database_error() { + let err = r2d2_sqlite::rusqlite::Error::InvalidQuery; + + let err: key::Error = err.into(); + + assert!(matches!(err, key::Error::KeyVerificationError { .. })); + } + } } From 3d89c7f8fcfd55f44692ec3163fab032426a72b7 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 4 Feb 2025 13:20:04 +0000 Subject: [PATCH 18/18] fix: [#1231] lint errors --- packages/tracker-core/src/authentication/service.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/tracker-core/src/authentication/service.rs b/packages/tracker-core/src/authentication/service.rs index 98b8a398..5ca0a09e 100644 --- a/packages/tracker-core/src/authentication/service.rs +++ b/packages/tracker-core/src/authentication/service.rs @@ -195,7 +195,8 @@ mod tests { } #[tokio::test] - async fn it_should_not_authenticate_a_registered_but_expired_key_when_the_tracker_is_explicitly_configured_to_check_keys_expiration() { + async fn it_should_not_authenticate_a_registered_but_expired_key_when_the_tracker_is_explicitly_configured_to_check_keys_expiration( + ) { let config = Core { private: true, private_mode: Some(PrivateMode {