From ceb30747cec6372b5244f797ad032c7db8c03d6b Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 10 May 2024 12:49:57 +0100 Subject: [PATCH] refactor: [#852] enrich field types in HttpApi config struct --- packages/configuration/src/v1/tracker_api.rs | 23 ++++--- packages/test-helpers/src/configuration.rs | 4 +- src/bootstrap/jobs/http_tracker.rs | 12 ++-- src/bootstrap/jobs/mod.rs | 68 ++++++++------------ src/bootstrap/jobs/tracker_apis.rs | 7 +- src/servers/apis/server.rs | 7 +- src/servers/http/server.rs | 12 ++-- tests/servers/api/environment.rs | 8 +-- tests/servers/http/environment.rs | 9 +-- 9 files changed, 56 insertions(+), 94 deletions(-) diff --git a/packages/configuration/src/v1/tracker_api.rs b/packages/configuration/src/v1/tracker_api.rs index 8749478c..5089c496 100644 --- a/packages/configuration/src/v1/tracker_api.rs +++ b/packages/configuration/src/v1/tracker_api.rs @@ -1,7 +1,10 @@ use std::collections::HashMap; +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use serde::{Deserialize, Serialize}; -use serde_with::{serde_as, NoneAsEmptyString}; +use serde_with::serde_as; + +use crate::TslConfig; pub type AccessTokens = HashMap; @@ -15,19 +18,16 @@ pub struct HttpApi { /// The format is `ip:port`, for example `0.0.0.0:6969`. If you want to /// listen to all interfaces, use `0.0.0.0`. If you want the operating /// system to choose a random port, use port `0`. - pub bind_address: String, + pub bind_address: SocketAddr, /// Weather the HTTP API will use SSL or not. pub ssl_enabled: bool, - /// Path to the SSL certificate file. Only used if `ssl_enabled` is `true`. - #[serde_as(as = "NoneAsEmptyString")] - pub ssl_cert_path: Option, - /// Path to the SSL key file. Only used if `ssl_enabled` is `true`. - #[serde_as(as = "NoneAsEmptyString")] - pub ssl_key_path: Option, + /// TSL config. Only used if `ssl_enabled` is true. + #[serde(flatten)] + pub tsl_config: TslConfig, /// Access tokens for the HTTP API. The key is a label identifying the /// token and the value is the token itself. The token is used to /// authenticate the user. All tokens are valid for all endpoints and have - /// the all permissions. + /// all permissions. pub access_tokens: AccessTokens, } @@ -35,10 +35,9 @@ impl Default for HttpApi { fn default() -> Self { Self { enabled: true, - bind_address: String::from("127.0.0.1:1212"), + bind_address: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 1212), ssl_enabled: false, - ssl_cert_path: None, - ssl_key_path: None, + tsl_config: TslConfig::default(), access_tokens: [(String::from("admin"), String::from("MyAccessToken"))] .iter() .cloned() diff --git a/packages/test-helpers/src/configuration.rs b/packages/test-helpers/src/configuration.rs index e6f53f85..08f570dc 100644 --- a/packages/test-helpers/src/configuration.rs +++ b/packages/test-helpers/src/configuration.rs @@ -35,7 +35,7 @@ pub fn ephemeral() -> Configuration { // Ephemeral socket address for API let api_port = 0u16; config.http_api.enabled = true; - config.http_api.bind_address = format!("127.0.0.1:{}", &api_port); + config.http_api.bind_address = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), api_port); // Ephemeral socket address for Health Check API let health_check_api_port = 0u16; @@ -138,7 +138,7 @@ pub fn ephemeral_ipv6() -> Configuration { let ipv6 = SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0)), 0); - cfg.http_api.bind_address.clone_from(&ipv6.to_string()); + cfg.http_api.bind_address.clone_from(&ipv6); cfg.http_trackers[0].bind_address.clone_from(&ipv6); cfg.udp_trackers[0].bind_address = ipv6; diff --git a/src/bootstrap/jobs/http_tracker.rs b/src/bootstrap/jobs/http_tracker.rs index d9e8bdab..d8a976b9 100644 --- a/src/bootstrap/jobs/http_tracker.rs +++ b/src/bootstrap/jobs/http_tracker.rs @@ -18,7 +18,7 @@ use log::info; use tokio::task::JoinHandle; use torrust_tracker_configuration::HttpTracker; -use super::make_rust_tls_from_path_buf; +use super::make_rust_tls; use crate::core; use crate::servers::http::server::{HttpServer, Launcher}; use crate::servers::http::Version; @@ -42,13 +42,9 @@ pub async fn start_job( if config.enabled { let socket = config.bind_address; - let tls = make_rust_tls_from_path_buf( - config.ssl_enabled, - &config.tsl_config.ssl_cert_path, - &config.tsl_config.ssl_key_path, - ) - .await - .map(|tls| tls.expect("it should have a valid http tracker tls configuration")); + let tls = make_rust_tls(config.ssl_enabled, &config.tsl_config) + .await + .map(|tls| tls.expect("it should have a valid http tracker tls configuration")); match version { Version::V1 => Some(start_v1(socket, tls, tracker.clone(), form).await), diff --git a/src/bootstrap/jobs/mod.rs b/src/bootstrap/jobs/mod.rs index dd855f7c..d288989b 100644 --- a/src/bootstrap/jobs/mod.rs +++ b/src/bootstrap/jobs/mod.rs @@ -20,41 +20,13 @@ pub struct Started { pub address: std::net::SocketAddr, } -pub async fn make_rust_tls(enabled: bool, cert: &Option, key: &Option) -> Option> { +pub async fn make_rust_tls(enabled: bool, tsl_config: &TslConfig) -> Option> { if !enabled { info!("TLS not enabled"); return None; } - if let (Some(cert), Some(key)) = (cert, key) { - info!("Using https: cert path: {cert}."); - info!("Using https: key path: {key}."); - - Some( - RustlsConfig::from_pem_file(cert, key) - .await - .map_err(|err| Error::BadTlsConfig { - source: (Arc::new(err) as DynError).into(), - }), - ) - } else { - Some(Err(Error::MissingTlsConfig { - location: Location::caller(), - })) - } -} - -pub async fn make_rust_tls_from_path_buf( - enabled: bool, - cert: &Option, - key: &Option, -) -> Option> { - if !enabled { - info!("TLS not enabled"); - return None; - } - - if let (Some(cert), Some(key)) = (cert, key) { + if let (Some(cert), Some(key)) = (tsl_config.ssl_cert_path.clone(), tsl_config.ssl_key_path.clone()) { info!("Using https: cert path: {cert}."); info!("Using https: key path: {key}."); @@ -75,15 +47,23 @@ pub async fn make_rust_tls_from_path_buf( #[cfg(test)] mod tests { + use camino::Utf8PathBuf; + use torrust_tracker_configuration::TslConfig; + use super::make_rust_tls; #[tokio::test] async fn it_should_error_on_bad_tls_config() { - let (bad_cert_path, bad_key_path) = (Some("bad cert path".to_string()), Some("bad key path".to_string())); - let err = make_rust_tls(true, &bad_cert_path, &bad_key_path) - .await - .expect("tls_was_enabled") - .expect_err("bad_cert_and_key_files"); + let err = make_rust_tls( + true, + &TslConfig { + ssl_cert_path: Some(Utf8PathBuf::from("bad cert path")), + ssl_key_path: Some(Utf8PathBuf::from("bad key path")), + }, + ) + .await + .expect("tls_was_enabled") + .expect_err("bad_cert_and_key_files"); assert!(err .to_string() @@ -91,11 +71,17 @@ mod tests { } #[tokio::test] - async fn it_should_error_on_missing_tls_config() { - let err = make_rust_tls(true, &None, &None) - .await - .expect("tls_was_enabled") - .expect_err("missing_config"); + async fn it_should_error_on_missing_cert_or_key_paths() { + let err = make_rust_tls( + true, + &TslConfig { + ssl_cert_path: None, + ssl_key_path: None, + }, + ) + .await + .expect("tls_was_enabled") + .expect_err("missing_config"); assert_eq!(err.to_string(), "tls config missing"); } @@ -105,9 +91,9 @@ use std::panic::Location; use std::sync::Arc; use axum_server::tls_rustls::RustlsConfig; -use camino::Utf8PathBuf; use log::info; use thiserror::Error; +use torrust_tracker_configuration::TslConfig; use torrust_tracker_located_error::{DynError, LocatedError}; /// Error returned by the Bootstrap Process. diff --git a/src/bootstrap/jobs/tracker_apis.rs b/src/bootstrap/jobs/tracker_apis.rs index ffd7c740..120c960e 100644 --- a/src/bootstrap/jobs/tracker_apis.rs +++ b/src/bootstrap/jobs/tracker_apis.rs @@ -61,12 +61,9 @@ pub async fn start_job( version: Version, ) -> Option> { if config.enabled { - let bind_to = config - .bind_address - .parse::() - .expect("it should have a valid tracker api bind address"); + let bind_to = config.bind_address; - let tls = make_rust_tls(config.ssl_enabled, &config.ssl_cert_path, &config.ssl_key_path) + let tls = make_rust_tls(config.ssl_enabled, &config.tsl_config) .await .map(|tls| tls.expect("it should have a valid tracker api tls configuration")); diff --git a/src/servers/apis/server.rs b/src/servers/apis/server.rs index e7289055..9317d6ec 100644 --- a/src/servers/apis/server.rs +++ b/src/servers/apis/server.rs @@ -275,12 +275,9 @@ mod tests { let tracker = initialize_with_configuration(&cfg); - let bind_to = config - .bind_address - .parse::() - .expect("Tracker API bind_address invalid."); + let bind_to = config.bind_address; - let tls = make_rust_tls(config.ssl_enabled, &config.ssl_cert_path, &config.ssl_key_path) + let tls = make_rust_tls(config.ssl_enabled, &config.tsl_config) .await .map(|tls| tls.expect("tls config failed")); diff --git a/src/servers/http/server.rs b/src/servers/http/server.rs index a6f96634..7c8148f2 100644 --- a/src/servers/http/server.rs +++ b/src/servers/http/server.rs @@ -224,7 +224,7 @@ mod tests { use torrust_tracker_test_helpers::configuration::ephemeral_mode_public; use crate::bootstrap::app::initialize_with_configuration; - use crate::bootstrap::jobs::make_rust_tls_from_path_buf; + use crate::bootstrap::jobs::make_rust_tls; use crate::servers::http::server::{HttpServer, Launcher}; use crate::servers::registar::Registar; @@ -236,13 +236,9 @@ mod tests { let bind_to = config.bind_address; - let tls = make_rust_tls_from_path_buf( - config.ssl_enabled, - &config.tsl_config.ssl_cert_path, - &config.tsl_config.ssl_key_path, - ) - .await - .map(|tls| tls.expect("tls config failed")); + let tls = make_rust_tls(config.ssl_enabled, &config.tsl_config) + .await + .map(|tls| tls.expect("tls config failed")); let register = &Registar::default(); diff --git a/tests/servers/api/environment.rs b/tests/servers/api/environment.rs index dec4ccff..cacde8af 100644 --- a/tests/servers/api/environment.rs +++ b/tests/servers/api/environment.rs @@ -33,13 +33,9 @@ impl Environment { let config = Arc::new(configuration.http_api.clone()); - let bind_to = config - .bind_address - .parse::() - .expect("Tracker API bind_address invalid."); + let bind_to = config.bind_address; - let tls = block_on(make_rust_tls(config.ssl_enabled, &config.ssl_cert_path, &config.ssl_key_path)) - .map(|tls| tls.expect("tls config failed")); + let tls = block_on(make_rust_tls(config.ssl_enabled, &config.tsl_config)).map(|tls| tls.expect("tls config failed")); let server = ApiServer::new(Launcher::new(bind_to, tls)); diff --git a/tests/servers/http/environment.rs b/tests/servers/http/environment.rs index e662cea7..61837c40 100644 --- a/tests/servers/http/environment.rs +++ b/tests/servers/http/environment.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use futures::executor::block_on; use torrust_tracker::bootstrap::app::initialize_with_configuration; -use torrust_tracker::bootstrap::jobs::make_rust_tls_from_path_buf; +use torrust_tracker::bootstrap::jobs::make_rust_tls; use torrust_tracker::core::Tracker; use torrust_tracker::servers::http::server::{HttpServer, Launcher, Running, Stopped}; use torrust_tracker::servers::registar::Registar; @@ -33,12 +33,7 @@ impl Environment { let bind_to = config.bind_address; - let tls = block_on(make_rust_tls_from_path_buf( - config.ssl_enabled, - &config.tsl_config.ssl_cert_path, - &config.tsl_config.ssl_key_path, - )) - .map(|tls| tls.expect("tls config failed")); + let tls = block_on(make_rust_tls(config.ssl_enabled, &config.tsl_config)).map(|tls| tls.expect("tls config failed")); let server = HttpServer::new(Launcher::new(bind_to, tls));