From 12c8cf9916492962e1be19f6aae7fc8aa826ece9 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 5 Mar 2023 11:39:49 +0000 Subject: [PATCH 1/4] refactor: ExpiringKey has always an expiration date Adter intruducing the `KeyId` we no longer needed to have keys with no expiration date. A key without an expiration date is a `KeyId`. So all `ExpiraingKeys` have an expiration date. --- src/apis/resources/auth_key.rs | 19 ++++--- src/databases/mysql.rs | 6 +-- src/databases/sqlite.rs | 6 +-- src/tracker/auth.rs | 98 +++++----------------------------- src/tracker/mod.rs | 2 +- 5 files changed, 32 insertions(+), 99 deletions(-) diff --git a/src/apis/resources/auth_key.rs b/src/apis/resources/auth_key.rs index e9989ca75..289e704b6 100644 --- a/src/apis/resources/auth_key.rs +++ b/src/apis/resources/auth_key.rs @@ -7,17 +7,20 @@ use crate::tracker::auth::{self, KeyId}; #[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] pub struct AuthKey { - pub key: String, // todo: rename to `id` - pub valid_until: Option, + pub key: String, // todo: rename to `id` (API breaking change!) + pub valid_until: Option, // todo: `auth::ExpiringKey` has now always a value (API breaking change!) } impl From for auth::ExpiringKey { fn from(auth_key_resource: AuthKey) -> Self { + let valid_until = match auth_key_resource.valid_until { + Some(valid_until) => DurationSinceUnixEpoch::from_secs(valid_until), + None => DurationSinceUnixEpoch::from_secs(0), + }; + auth::ExpiringKey { id: auth_key_resource.key.parse::().unwrap(), - valid_until: auth_key_resource - .valid_until - .map(|valid_until| DurationSinceUnixEpoch::new(valid_until, 0)), + valid_until, } } } @@ -26,7 +29,7 @@ impl From for AuthKey { fn from(auth_key: auth::ExpiringKey) -> Self { AuthKey { key: auth_key.id.to_string(), - valid_until: auth_key.valid_until.map(|valid_until| valid_until.as_secs()), + valid_until: Some(auth_key.valid_until.as_secs()), } } } @@ -52,7 +55,7 @@ mod tests { auth::ExpiringKey::from(auth_key_resource), auth::ExpiringKey { id: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line - valid_until: Some(Current::add(&Duration::new(duration_in_secs, 0)).unwrap()) + valid_until: Current::add(&Duration::new(duration_in_secs, 0)).unwrap() } ); } @@ -63,7 +66,7 @@ mod tests { let auth_key = auth::ExpiringKey { id: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line - valid_until: Some(Current::add(&Duration::new(duration_in_secs, 0)).unwrap()), + valid_until: Current::add(&Duration::new(duration_in_secs, 0)).unwrap(), }; assert_eq!( diff --git a/src/databases/mysql.rs b/src/databases/mysql.rs index 0d545aaa9..cbd5f3df9 100644 --- a/src/databases/mysql.rs +++ b/src/databases/mysql.rs @@ -118,7 +118,7 @@ impl Database for Mysql { "SELECT `key`, valid_until FROM `keys`", |(key, valid_until): (String, i64)| auth::ExpiringKey { id: key.parse::().unwrap(), - valid_until: Some(Duration::from_secs(valid_until.unsigned_abs())), + valid_until: Duration::from_secs(valid_until.unsigned_abs()), }, )?; @@ -193,7 +193,7 @@ impl Database for Mysql { Ok(key.map(|(key, expiry)| auth::ExpiringKey { id: key.parse::().unwrap(), - valid_until: Some(Duration::from_secs(expiry.unsigned_abs())), + valid_until: Duration::from_secs(expiry.unsigned_abs()), })) } @@ -201,7 +201,7 @@ impl Database for Mysql { let mut conn = self.pool.get().map_err(|e| (e, DRIVER))?; let key = auth_key.id.to_string(); - let valid_until = auth_key.valid_until.unwrap_or(Duration::ZERO).as_secs().to_string(); + let valid_until = auth_key.valid_until.as_secs().to_string(); conn.exec_drop( "INSERT INTO `keys` (`key`, valid_until) VALUES (:key, :valid_until)", diff --git a/src/databases/sqlite.rs b/src/databases/sqlite.rs index ab0addf4b..974f172e0 100644 --- a/src/databases/sqlite.rs +++ b/src/databases/sqlite.rs @@ -113,7 +113,7 @@ impl Database for Sqlite { Ok(auth::ExpiringKey { id: key.parse::().unwrap(), - valid_until: Some(DurationSinceUnixEpoch::from_secs(valid_until.unsigned_abs())), + valid_until: DurationSinceUnixEpoch::from_secs(valid_until.unsigned_abs()), }) })?; @@ -214,7 +214,7 @@ impl Database for Sqlite { let id: String = f.get(0).unwrap(); auth::ExpiringKey { id: id.parse::().unwrap(), - valid_until: Some(DurationSinceUnixEpoch::from_secs(expiry.unsigned_abs())), + valid_until: DurationSinceUnixEpoch::from_secs(expiry.unsigned_abs()), } })) } @@ -224,7 +224,7 @@ impl Database for Sqlite { let insert = conn.execute( "INSERT INTO keys (key, valid_until) VALUES (?1, ?2)", - [auth_key.id.to_string(), auth_key.valid_until.unwrap().as_secs().to_string()], + [auth_key.id.to_string(), auth_key.valid_until.as_secs().to_string()], )?; if insert == 0 { diff --git a/src/tracker/auth.rs b/src/tracker/auth.rs index 01de7a619..f8e1b3440 100644 --- a/src/tracker/auth.rs +++ b/src/tracker/auth.rs @@ -30,7 +30,7 @@ pub fn generate(lifetime: Duration) -> ExpiringKey { ExpiringKey { id: random_id.parse::().unwrap(), - valid_until: Some(Current::add(&lifetime).unwrap()), + valid_until: Current::add(&lifetime).unwrap(), } } @@ -42,30 +42,19 @@ pub fn generate(lifetime: Duration) -> ExpiringKey { pub fn verify(auth_key: &ExpiringKey) -> Result<(), Error> { let current_time: DurationSinceUnixEpoch = Current::now(); - match auth_key.valid_until { - Some(valid_until) => { - if valid_until < current_time { - Err(Error::KeyExpired { - location: Location::caller(), - }) - } else { - Ok(()) - } - } - None => Err(Error::UnableToReadKey { + if auth_key.valid_until < current_time { + Err(Error::KeyExpired { location: Location::caller(), - key_id: Box::new(auth_key.id.clone()), - }), + }) + } else { + Ok(()) } } #[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)] pub struct ExpiringKey { pub id: KeyId, - // todo: we can remove the `Option`. An `ExpiringKey` that does not expire - // is a `KeyId`. In other words, all `ExpiringKeys` must have an - // expiration time. - pub valid_until: Option, + pub valid_until: DurationSinceUnixEpoch, } impl std::fmt::Display for ExpiringKey { @@ -74,54 +63,18 @@ impl std::fmt::Display for ExpiringKey { f, "key: `{}`, valid until `{}`", self.id, - match self.valid_until { - Some(duration) => format!( - "{}", - DateTime::::from_utc( - NaiveDateTime::from_timestamp( - i64::try_from(duration.as_secs()).expect("Overflow of i64 seconds, very future!"), - duration.subsec_nanos(), - ), - Utc - ) + DateTime::::from_utc( + NaiveDateTime::from_timestamp( + i64::try_from(self.valid_until.as_secs()).expect("Overflow of i64 seconds, very future!"), + self.valid_until.subsec_nanos(), ), - None => "Empty!?".to_string(), - } + Utc + ) ) } } impl ExpiringKey { - /// # Panics - /// - /// Will panic if bytes cannot be converted into a valid `KeyId`. - #[must_use] - pub fn from_buffer(key_buffer: [u8; AUTH_KEY_LENGTH]) -> Option { - if let Ok(key) = String::from_utf8(Vec::from(key_buffer)) { - Some(ExpiringKey { - id: key.parse::().unwrap(), - valid_until: None, - }) - } else { - None - } - } - - /// # Panics - /// - /// Will panic if string cannot be converted into a valid `KeyId`. - #[must_use] - pub fn from_string(key: &str) -> Option { - if key.len() == AUTH_KEY_LENGTH { - Some(ExpiringKey { - id: key.parse::().unwrap(), - valid_until: None, - }) - } else { - None - } - } - #[must_use] pub fn id(&self) -> KeyId { self.id.clone() @@ -176,30 +129,7 @@ mod tests { use std::time::Duration; use crate::protocol::clock::{Current, StoppedTime}; - use crate::tracker::auth::{self, KeyId}; - - #[test] - fn auth_key_from_buffer() { - let auth_key = auth::ExpiringKey::from_buffer([ - 89, 90, 83, 108, 52, 108, 77, 90, 117, 112, 82, 117, 79, 112, 83, 82, 67, 51, 107, 114, 73, 75, 82, 53, 66, 80, 66, - 49, 52, 110, 114, 74, - ]); - - assert!(auth_key.is_some()); - assert_eq!( - auth_key.unwrap().id, - "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".parse::().unwrap() - ); - } - - #[test] - fn auth_key_from_string() { - let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; - let auth_key = auth::ExpiringKey::from_string(key_string); - - assert!(auth_key.is_some()); - assert_eq!(auth_key.unwrap().id, key_string.parse::().unwrap()); - } + use crate::tracker::auth; #[test] fn auth_key_id_from_string() { diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index fbda95354..0d04868a8 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -1147,7 +1147,7 @@ mod tests { let key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); - assert_eq!(key.valid_until.unwrap(), Duration::from_secs(100)); + assert_eq!(key.valid_until, Duration::from_secs(100)); } #[tokio::test] From c7015fad532983bb4661d510c3dd5cdb46c45acd Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 5 Mar 2023 11:46:26 +0000 Subject: [PATCH 2/4] refactor: rename struct KeyId to Key There is no longer a clonflict with the `ExpiringKey` strcut that was also called `Key`. --- src/apis/handlers.rs | 4 ++-- src/apis/resources/auth_key.rs | 12 ++++++------ src/databases/mysql.rs | 6 +++--- src/databases/sqlite.rs | 6 +++--- src/http/axum_implementation/extractors/key.rs | 6 +++--- src/http/warp_implementation/filters.rs | 8 ++++---- src/http/warp_implementation/handlers.rs | 8 ++++---- src/tracker/auth.rs | 14 +++++++------- src/tracker/error.rs | 2 +- src/tracker/mod.rs | 16 ++++++++-------- tests/http/client.rs | 6 +++--- tests/http/connection_info.rs | 4 ++-- tests/http_tracker.rs | 16 ++++++++-------- tests/tracker_api.rs | 4 ++-- 14 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/apis/handlers.rs b/src/apis/handlers.rs index f7b5e562c..652f491e5 100644 --- a/src/apis/handlers.rs +++ b/src/apis/handlers.rs @@ -17,7 +17,7 @@ use crate::apis::resources::auth_key::AuthKey; use crate::apis::resources::stats::Stats; use crate::apis::resources::torrent::ListItem; use crate::protocol::info_hash::InfoHash; -use crate::tracker::auth::KeyId; +use crate::tracker::auth::Key; use crate::tracker::services::statistics::get_metrics; use crate::tracker::services::torrent::{get_torrent_info, get_torrents, Pagination}; use crate::tracker::Tracker; @@ -107,7 +107,7 @@ pub async fn delete_auth_key_handler( State(tracker): State>, Path(seconds_valid_or_key): Path, ) -> Response { - match KeyId::from_str(&seconds_valid_or_key.0) { + match Key::from_str(&seconds_valid_or_key.0) { Err(_) => invalid_auth_key_param_response(&seconds_valid_or_key.0), Ok(key_id) => match tracker.remove_auth_key(&key_id.to_string()).await { Ok(_) => ok_response(), diff --git a/src/apis/resources/auth_key.rs b/src/apis/resources/auth_key.rs index 289e704b6..954e633d0 100644 --- a/src/apis/resources/auth_key.rs +++ b/src/apis/resources/auth_key.rs @@ -3,11 +3,11 @@ use std::convert::From; use serde::{Deserialize, Serialize}; use crate::protocol::clock::DurationSinceUnixEpoch; -use crate::tracker::auth::{self, KeyId}; +use crate::tracker::auth::{self, Key}; #[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] pub struct AuthKey { - pub key: String, // todo: rename to `id` (API breaking change!) + pub key: String, // todo: rename to `id` (API breaking change!) pub valid_until: Option, // todo: `auth::ExpiringKey` has now always a value (API breaking change!) } @@ -19,7 +19,7 @@ impl From for auth::ExpiringKey { }; auth::ExpiringKey { - id: auth_key_resource.key.parse::().unwrap(), + id: auth_key_resource.key.parse::().unwrap(), valid_until, } } @@ -40,7 +40,7 @@ mod tests { use super::AuthKey; use crate::protocol::clock::{Current, TimeNow}; - use crate::tracker::auth::{self, KeyId}; + use crate::tracker::auth::{self, Key}; #[test] fn it_should_be_convertible_into_an_auth_key() { @@ -54,7 +54,7 @@ mod tests { assert_eq!( auth::ExpiringKey::from(auth_key_resource), auth::ExpiringKey { - id: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line + id: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line valid_until: Current::add(&Duration::new(duration_in_secs, 0)).unwrap() } ); @@ -65,7 +65,7 @@ mod tests { let duration_in_secs = 60; let auth_key = auth::ExpiringKey { - id: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line + id: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line valid_until: Current::add(&Duration::new(duration_in_secs, 0)).unwrap(), }; diff --git a/src/databases/mysql.rs b/src/databases/mysql.rs index cbd5f3df9..00865d026 100644 --- a/src/databases/mysql.rs +++ b/src/databases/mysql.rs @@ -12,7 +12,7 @@ use super::driver::Driver; use crate::databases::{Database, Error}; use crate::protocol::common::AUTH_KEY_LENGTH; use crate::protocol::info_hash::InfoHash; -use crate::tracker::auth::{self, KeyId}; +use crate::tracker::auth::{self, Key}; const DRIVER: Driver = Driver::MySQL; @@ -117,7 +117,7 @@ impl Database for Mysql { let keys = conn.query_map( "SELECT `key`, valid_until FROM `keys`", |(key, valid_until): (String, i64)| auth::ExpiringKey { - id: key.parse::().unwrap(), + id: key.parse::().unwrap(), valid_until: Duration::from_secs(valid_until.unsigned_abs()), }, )?; @@ -192,7 +192,7 @@ impl Database for Mysql { let key = query?; Ok(key.map(|(key, expiry)| auth::ExpiringKey { - id: key.parse::().unwrap(), + id: key.parse::().unwrap(), valid_until: Duration::from_secs(expiry.unsigned_abs()), })) } diff --git a/src/databases/sqlite.rs b/src/databases/sqlite.rs index 974f172e0..6c5b9f600 100644 --- a/src/databases/sqlite.rs +++ b/src/databases/sqlite.rs @@ -9,7 +9,7 @@ use super::driver::Driver; use crate::databases::{Database, Error}; use crate::protocol::clock::DurationSinceUnixEpoch; use crate::protocol::info_hash::InfoHash; -use crate::tracker::auth::{self, KeyId}; +use crate::tracker::auth::{self, Key}; const DRIVER: Driver = Driver::Sqlite3; @@ -112,7 +112,7 @@ impl Database for Sqlite { let valid_until: i64 = row.get(1)?; Ok(auth::ExpiringKey { - id: key.parse::().unwrap(), + id: key.parse::().unwrap(), valid_until: DurationSinceUnixEpoch::from_secs(valid_until.unsigned_abs()), }) })?; @@ -213,7 +213,7 @@ impl Database for Sqlite { let expiry: i64 = f.get(1).unwrap(); let id: String = f.get(0).unwrap(); auth::ExpiringKey { - id: id.parse::().unwrap(), + id: id.parse::().unwrap(), valid_until: DurationSinceUnixEpoch::from_secs(expiry.unsigned_abs()), } })) diff --git a/src/http/axum_implementation/extractors/key.rs b/src/http/axum_implementation/extractors/key.rs index 6cc2f13e8..ecdc9d801 100644 --- a/src/http/axum_implementation/extractors/key.rs +++ b/src/http/axum_implementation/extractors/key.rs @@ -7,9 +7,9 @@ use axum::response::{IntoResponse, Response}; use crate::http::axum_implementation::handlers::auth::{self, KeyIdParam}; use crate::http::axum_implementation::responses; -use crate::tracker::auth::KeyId; +use crate::tracker::auth::Key; -pub struct ExtractKeyId(pub KeyId); +pub struct ExtractKeyId(pub Key); #[async_trait] impl FromRequestParts for ExtractKeyId @@ -21,7 +21,7 @@ where async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { match Path::::from_request_parts(parts, state).await { Ok(key_id_param) => { - let Ok(key_id) = key_id_param.0.value().parse::() else { + let Ok(key_id) = key_id_param.0.value().parse::() else { return Err(responses::error::Error::from( auth::Error::InvalidKeyFormat { location: Location::caller() diff --git a/src/http/warp_implementation/filters.rs b/src/http/warp_implementation/filters.rs index eb7abcd4d..a3000bfaa 100644 --- a/src/http/warp_implementation/filters.rs +++ b/src/http/warp_implementation/filters.rs @@ -12,7 +12,7 @@ use super::{request, WebResult}; use crate::http::percent_encoding::{percent_decode_info_hash, percent_decode_peer_id}; use crate::protocol::common::MAX_SCRAPE_TORRENTS; use crate::protocol::info_hash::InfoHash; -use crate::tracker::auth::KeyId; +use crate::tracker::auth::Key; use crate::tracker::{self, peer}; /// Pass Arc along @@ -37,16 +37,16 @@ pub fn with_peer_id() -> impl Filter + /// Pass Arc along #[must_use] -pub fn with_auth_key_id() -> impl Filter,), Error = Infallible> + Clone { +pub fn with_auth_key_id() -> impl Filter,), Error = Infallible> + Clone { warp::path::param::() .map(|key: String| { - let key_id = KeyId::from_str(&key); + let key_id = Key::from_str(&key); match key_id { Ok(id) => Some(id), Err(_) => None, } }) - .or_else(|_| async { Ok::<(Option,), Infallible>((None,)) }) + .or_else(|_| async { Ok::<(Option,), Infallible>((None,)) }) } /// Check for `PeerAddress` diff --git a/src/http/warp_implementation/handlers.rs b/src/http/warp_implementation/handlers.rs index b803a594f..4a64259bb 100644 --- a/src/http/warp_implementation/handlers.rs +++ b/src/http/warp_implementation/handlers.rs @@ -12,7 +12,7 @@ use super::error::Error; use super::{request, response, WebResult}; use crate::http::warp_implementation::peer_builder; use crate::protocol::info_hash::InfoHash; -use crate::tracker::auth::KeyId; +use crate::tracker::auth::Key; use crate::tracker::{self, auth, peer, statistics, torrent}; /// Authenticate `InfoHash` using optional `auth::Key` @@ -22,7 +22,7 @@ use crate::tracker::{self, auth, peer, statistics, torrent}; /// Will return `ServerError` that wraps the `tracker::error::Error` if unable to `authenticate_request`. pub async fn authenticate( info_hash: &InfoHash, - auth_key_id: &Option, + auth_key_id: &Option, tracker: Arc, ) -> Result<(), Error> { tracker @@ -38,7 +38,7 @@ pub async fn authenticate( /// Will return `warp::Rejection` that wraps the `ServerError` if unable to `send_announce_response`. pub async fn handle_announce( announce_request: request::Announce, - auth_key_id: Option, + auth_key_id: Option, tracker: Arc, ) -> WebResult { debug!("http announce request: {:#?}", announce_request); @@ -78,7 +78,7 @@ pub async fn handle_announce( /// Will return `warp::Rejection` that wraps the `ServerError` if unable to `send_scrape_response`. pub async fn handle_scrape( scrape_request: request::Scrape, - auth_key_id: Option, + auth_key_id: Option, tracker: Arc, ) -> WebResult { let mut files: HashMap = HashMap::new(); diff --git a/src/tracker/auth.rs b/src/tracker/auth.rs index f8e1b3440..2f65b2bcb 100644 --- a/src/tracker/auth.rs +++ b/src/tracker/auth.rs @@ -29,7 +29,7 @@ pub fn generate(lifetime: Duration) -> ExpiringKey { debug!("Generated key: {}, valid for: {:?} seconds", random_id, lifetime); ExpiringKey { - id: random_id.parse::().unwrap(), + id: random_id.parse::().unwrap(), valid_until: Current::add(&lifetime).unwrap(), } } @@ -53,7 +53,7 @@ pub fn verify(auth_key: &ExpiringKey) -> Result<(), Error> { #[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)] pub struct ExpiringKey { - pub id: KeyId, + pub id: Key, pub valid_until: DurationSinceUnixEpoch, } @@ -76,18 +76,18 @@ impl std::fmt::Display for ExpiringKey { impl ExpiringKey { #[must_use] - pub fn id(&self) -> KeyId { + pub fn id(&self) -> Key { self.id.clone() } } #[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone, Display, Hash)] -pub struct KeyId(String); +pub struct Key(String); #[derive(Debug, PartialEq, Eq)] pub struct ParseKeyIdError; -impl FromStr for KeyId { +impl FromStr for Key { type Err = ParseKeyIdError; fn from_str(s: &str) -> Result { @@ -109,7 +109,7 @@ pub enum Error { #[error("Failed to read key: {key_id}, {location}")] UnableToReadKey { location: &'static Location<'static>, - key_id: Box, + key_id: Box, }, #[error("Key has expired, {location}")] KeyExpired { location: &'static Location<'static> }, @@ -134,7 +134,7 @@ mod tests { #[test] fn auth_key_id_from_string() { let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; - let auth_key_id = auth::KeyId::from_str(key_string); + let auth_key_id = auth::Key::from_str(key_string); assert!(auth_key_id.is_ok()); assert_eq!(auth_key_id.unwrap().to_string(), key_string); diff --git a/src/tracker/error.rs b/src/tracker/error.rs index acc85a1c2..f03f4b3e5 100644 --- a/src/tracker/error.rs +++ b/src/tracker/error.rs @@ -6,7 +6,7 @@ use crate::located_error::LocatedError; pub enum Error { #[error("The supplied key: {key_id:?}, is not valid: {source}")] PeerKeyNotValid { - key_id: super::auth::KeyId, + key_id: super::auth::Key, source: LocatedError<'static, dyn std::error::Error + Send + Sync>, }, #[error("The peer is not authenticated, {location}")] diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index 0d04868a8..448905ef7 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -16,7 +16,7 @@ use std::time::Duration; use tokio::sync::mpsc::error::SendError; use tokio::sync::{RwLock, RwLockReadGuard}; -use self::auth::KeyId; +use self::auth::Key; use self::error::Error; use self::peer::Peer; use self::torrent::{SwarmMetadata, SwarmStats}; @@ -28,7 +28,7 @@ use crate::protocol::info_hash::InfoHash; pub struct Tracker { pub config: Arc, mode: mode::Mode, - keys: RwLock>, + keys: RwLock>, whitelist: RwLock>, torrents: RwLock>, stats_event_sender: Option>, @@ -204,14 +204,14 @@ impl Tracker { pub async fn remove_auth_key(&self, key: &str) -> Result<(), databases::error::Error> { // todo: change argument `key: &str` to `key_id: &KeyId` self.database.remove_key_from_keys(key).await?; - self.keys.write().await.remove(&key.parse::().unwrap()); + self.keys.write().await.remove(&key.parse::().unwrap()); Ok(()) } /// # Errors /// /// Will return a `key::Error` if unable to get any `auth_key`. - pub async fn verify_auth_key(&self, key_id: &KeyId) -> Result<(), auth::Error> { + pub async fn verify_auth_key(&self, key_id: &Key) -> Result<(), auth::Error> { // code-review: this function is public only because it's used in a test. // We should change the test and make it private. match self.keys.read().await.get(key_id) { @@ -324,7 +324,7 @@ impl Tracker { /// Will return a `torrent::Error::PeerNotAuthenticated` if the `key` is `None`. /// /// Will return a `torrent::Error::TorrentNotWhitelisted` if the the Tracker is in listed mode and the `info_hash` is not whitelisted. - pub async fn authenticate_request(&self, info_hash: &InfoHash, key: &Option) -> Result<(), Error> { + pub async fn authenticate_request(&self, info_hash: &InfoHash, key: &Option) -> Result<(), Error> { // todo: this is a deprecated method. // We're splitting authentication and authorization responsibilities. // Use `authenticate` and `authorize` instead. @@ -371,7 +371,7 @@ impl Tracker { /// # Errors /// /// Will return an error if the the authentication key cannot be verified. - pub async fn authenticate(&self, key_id: &KeyId) -> Result<(), auth::Error> { + pub async fn authenticate(&self, key_id: &Key) -> Result<(), auth::Error> { if self.is_private() { self.verify_auth_key(key_id).await } else { @@ -1165,7 +1165,7 @@ mod tests { async fn it_should_fail_authenticating_a_peer_when_it_uses_an_unregistered_key() { let tracker = private_tracker(); - let unregistered_key_id = auth::KeyId::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + let unregistered_key_id = auth::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); let result = tracker.authenticate(&unregistered_key_id).await; @@ -1187,7 +1187,7 @@ mod tests { async fn it_should_fail_verifying_an_unregistered_authentication_key() { let tracker = private_tracker(); - let unregistered_key_id = auth::KeyId::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + let unregistered_key_id = auth::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); assert!(tracker.verify_auth_key(&unregistered_key_id).await.is_err()); } diff --git a/tests/http/client.rs b/tests/http/client.rs index b59cf2ac6..fa5fd5d16 100644 --- a/tests/http/client.rs +++ b/tests/http/client.rs @@ -1,7 +1,7 @@ use std::net::IpAddr; use reqwest::{Client as ReqwestClient, Response}; -use torrust_tracker::tracker::auth::KeyId; +use torrust_tracker::tracker::auth::Key; use super::connection_info::ConnectionInfo; use super::requests::announce::{self, Query}; @@ -11,7 +11,7 @@ use super::requests::scrape; pub struct Client { connection_info: ConnectionInfo, reqwest_client: ReqwestClient, - key_id: Option, + key_id: Option, } /// URL components in this context: @@ -40,7 +40,7 @@ impl Client { } } - pub fn authenticated(connection_info: ConnectionInfo, key_id: KeyId) -> Self { + pub fn authenticated(connection_info: ConnectionInfo, key_id: Key) -> Self { Self { connection_info, reqwest_client: reqwest::Client::builder().build().unwrap(), diff --git a/tests/http/connection_info.rs b/tests/http/connection_info.rs index fb1dbf64e..eedaa73f0 100644 --- a/tests/http/connection_info.rs +++ b/tests/http/connection_info.rs @@ -1,9 +1,9 @@ -use torrust_tracker::tracker::auth::KeyId; +use torrust_tracker::tracker::auth::Key; #[derive(Clone, Debug)] pub struct ConnectionInfo { pub bind_address: String, - pub key_id: Option, + pub key_id: Option, } impl ConnectionInfo { diff --git a/tests/http_tracker.rs b/tests/http_tracker.rs index 2360df9ab..b1b90f923 100644 --- a/tests/http_tracker.rs +++ b/tests/http_tracker.rs @@ -1083,7 +1083,7 @@ mod warp_http_tracker_server { use torrust_tracker::http::Version; use torrust_tracker::protocol::info_hash::InfoHash; - use torrust_tracker::tracker::auth::KeyId; + use torrust_tracker::tracker::auth::Key; use crate::http::asserts::assert_is_announce_response; use crate::http::asserts_warp::{ @@ -1128,7 +1128,7 @@ mod warp_http_tracker_server { let http_tracker_server = start_private_http_tracker(Version::Warp).await; // The tracker does not have this key - let unregistered_key_id = KeyId::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + let unregistered_key_id = Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); let response = Client::authenticated(http_tracker_server.get_connection_info(), unregistered_key_id) .announce(&QueryBuilder::default().query()) @@ -1145,7 +1145,7 @@ mod warp_http_tracker_server { use torrust_tracker::http::Version; use torrust_tracker::protocol::info_hash::InfoHash; - use torrust_tracker::tracker::auth::KeyId; + use torrust_tracker::tracker::auth::Key; use torrust_tracker::tracker::peer; use crate::common::fixtures::PeerBuilder; @@ -1242,7 +1242,7 @@ mod warp_http_tracker_server { ) .await; - let false_key_id: KeyId = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".parse().unwrap(); + let false_key_id: Key = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".parse().unwrap(); let response = Client::authenticated(http_tracker.get_connection_info(), false_key_id) .scrape( @@ -2396,7 +2396,7 @@ mod axum_http_tracker_server { use torrust_tracker::http::Version; use torrust_tracker::protocol::info_hash::InfoHash; - use torrust_tracker::tracker::auth::KeyId; + use torrust_tracker::tracker::auth::Key; use crate::http::asserts::{assert_authentication_error_response, assert_is_announce_response}; use crate::http::client::Client; @@ -2453,7 +2453,7 @@ mod axum_http_tracker_server { let http_tracker_server = start_private_http_tracker(Version::Axum).await; // The tracker does not have this key - let unregistered_key_id = KeyId::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + let unregistered_key_id = Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); let response = Client::authenticated(http_tracker_server.get_connection_info(), unregistered_key_id) .announce(&QueryBuilder::default().query()) @@ -2470,7 +2470,7 @@ mod axum_http_tracker_server { use torrust_tracker::http::Version; use torrust_tracker::protocol::info_hash::InfoHash; - use torrust_tracker::tracker::auth::KeyId; + use torrust_tracker::tracker::auth::Key; use torrust_tracker::tracker::peer; use crate::common::fixtures::PeerBuilder; @@ -2583,7 +2583,7 @@ mod axum_http_tracker_server { ) .await; - let false_key_id: KeyId = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".parse().unwrap(); + let false_key_id: Key = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".parse().unwrap(); let response = Client::authenticated(http_tracker.get_connection_info(), false_key_id) .scrape( diff --git a/tests/tracker_api.rs b/tests/tracker_api.rs index bec22e2b4..2c59cd8fb 100644 --- a/tests/tracker_api.rs +++ b/tests/tracker_api.rs @@ -638,7 +638,7 @@ mod tracker_apis { mod for_key_resources { use std::time::Duration; - use torrust_tracker::tracker::auth::KeyId; + use torrust_tracker::tracker::auth::Key; use crate::api::asserts::{ assert_auth_key_utf8, assert_failed_to_delete_key, assert_failed_to_generate_key, assert_failed_to_reload_keys, @@ -665,7 +665,7 @@ mod tracker_apis { // Verify the key with the tracker assert!(api_server .tracker - .verify_auth_key(&auth_key_resource.key.parse::().unwrap()) + .verify_auth_key(&auth_key_resource.key.parse::().unwrap()) .await .is_ok()); } From 7b690a4c6a63872ce411adbaf198b43381dcb4c4 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 5 Mar 2023 11:59:45 +0000 Subject: [PATCH 3/4] refactor: rename ExpiringKey::id to ExpiringKey:key --- src/apis/resources/auth_key.rs | 8 ++++---- src/databases/mysql.rs | 6 +++--- src/databases/sqlite.rs | 6 +++--- src/tracker/auth.rs | 8 ++++---- src/tracker/mod.rs | 4 ++-- tests/tracker_api.rs | 8 ++++---- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/apis/resources/auth_key.rs b/src/apis/resources/auth_key.rs index 954e633d0..72ef32a95 100644 --- a/src/apis/resources/auth_key.rs +++ b/src/apis/resources/auth_key.rs @@ -19,7 +19,7 @@ impl From for auth::ExpiringKey { }; auth::ExpiringKey { - id: auth_key_resource.key.parse::().unwrap(), + key: auth_key_resource.key.parse::().unwrap(), valid_until, } } @@ -28,7 +28,7 @@ impl From for auth::ExpiringKey { impl From for AuthKey { fn from(auth_key: auth::ExpiringKey) -> Self { AuthKey { - key: auth_key.id.to_string(), + key: auth_key.key.to_string(), valid_until: Some(auth_key.valid_until.as_secs()), } } @@ -54,7 +54,7 @@ mod tests { assert_eq!( auth::ExpiringKey::from(auth_key_resource), auth::ExpiringKey { - id: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line + key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line valid_until: Current::add(&Duration::new(duration_in_secs, 0)).unwrap() } ); @@ -65,7 +65,7 @@ mod tests { let duration_in_secs = 60; let auth_key = auth::ExpiringKey { - id: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line + key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line valid_until: Current::add(&Duration::new(duration_in_secs, 0)).unwrap(), }; diff --git a/src/databases/mysql.rs b/src/databases/mysql.rs index 00865d026..4bb28f050 100644 --- a/src/databases/mysql.rs +++ b/src/databases/mysql.rs @@ -117,7 +117,7 @@ impl Database for Mysql { let keys = conn.query_map( "SELECT `key`, valid_until FROM `keys`", |(key, valid_until): (String, i64)| auth::ExpiringKey { - id: key.parse::().unwrap(), + key: key.parse::().unwrap(), valid_until: Duration::from_secs(valid_until.unsigned_abs()), }, )?; @@ -192,7 +192,7 @@ impl Database for Mysql { let key = query?; Ok(key.map(|(key, expiry)| auth::ExpiringKey { - id: key.parse::().unwrap(), + key: key.parse::().unwrap(), valid_until: Duration::from_secs(expiry.unsigned_abs()), })) } @@ -200,7 +200,7 @@ impl Database for Mysql { async fn add_key_to_keys(&self, auth_key: &auth::ExpiringKey) -> Result { let mut conn = self.pool.get().map_err(|e| (e, DRIVER))?; - let key = auth_key.id.to_string(); + let key = auth_key.key.to_string(); let valid_until = auth_key.valid_until.as_secs().to_string(); conn.exec_drop( diff --git a/src/databases/sqlite.rs b/src/databases/sqlite.rs index 6c5b9f600..8fac09e47 100644 --- a/src/databases/sqlite.rs +++ b/src/databases/sqlite.rs @@ -112,7 +112,7 @@ impl Database for Sqlite { let valid_until: i64 = row.get(1)?; Ok(auth::ExpiringKey { - id: key.parse::().unwrap(), + key: key.parse::().unwrap(), valid_until: DurationSinceUnixEpoch::from_secs(valid_until.unsigned_abs()), }) })?; @@ -213,7 +213,7 @@ impl Database for Sqlite { let expiry: i64 = f.get(1).unwrap(); let id: String = f.get(0).unwrap(); auth::ExpiringKey { - id: id.parse::().unwrap(), + key: id.parse::().unwrap(), valid_until: DurationSinceUnixEpoch::from_secs(expiry.unsigned_abs()), } })) @@ -224,7 +224,7 @@ impl Database for Sqlite { let insert = conn.execute( "INSERT INTO keys (key, valid_until) VALUES (?1, ?2)", - [auth_key.id.to_string(), auth_key.valid_until.as_secs().to_string()], + [auth_key.key.to_string(), auth_key.valid_until.as_secs().to_string()], )?; if insert == 0 { diff --git a/src/tracker/auth.rs b/src/tracker/auth.rs index 2f65b2bcb..09f324e2b 100644 --- a/src/tracker/auth.rs +++ b/src/tracker/auth.rs @@ -29,7 +29,7 @@ pub fn generate(lifetime: Duration) -> ExpiringKey { debug!("Generated key: {}, valid for: {:?} seconds", random_id, lifetime); ExpiringKey { - id: random_id.parse::().unwrap(), + key: random_id.parse::().unwrap(), valid_until: Current::add(&lifetime).unwrap(), } } @@ -53,7 +53,7 @@ pub fn verify(auth_key: &ExpiringKey) -> Result<(), Error> { #[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone)] pub struct ExpiringKey { - pub id: Key, + pub key: Key, pub valid_until: DurationSinceUnixEpoch, } @@ -62,7 +62,7 @@ impl std::fmt::Display for ExpiringKey { write!( f, "key: `{}`, valid until `{}`", - self.id, + self.key, DateTime::::from_utc( NaiveDateTime::from_timestamp( i64::try_from(self.valid_until.as_secs()).expect("Overflow of i64 seconds, very future!"), @@ -77,7 +77,7 @@ impl std::fmt::Display for ExpiringKey { impl ExpiringKey { #[must_use] pub fn id(&self) -> Key { - self.id.clone() + self.key.clone() } } diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index 448905ef7..34977b4de 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -190,7 +190,7 @@ impl Tracker { pub async fn generate_auth_key(&self, lifetime: Duration) -> Result { let auth_key = auth::generate(lifetime); self.database.add_key_to_keys(&auth_key).await?; - self.keys.write().await.insert(auth_key.id.clone(), auth_key.clone()); + self.keys.write().await.insert(auth_key.key.clone(), auth_key.clone()); Ok(auth_key) } @@ -233,7 +233,7 @@ impl Tracker { keys.clear(); for key in keys_from_database { - keys.insert(key.id.clone(), key); + keys.insert(key.key.clone(), key); } Ok(()) diff --git a/tests/tracker_api.rs b/tests/tracker_api.rs index 2c59cd8fb..600d26f2f 100644 --- a/tests/tracker_api.rs +++ b/tests/tracker_api.rs @@ -734,7 +734,7 @@ mod tracker_apis { .unwrap(); let response = Client::new(api_server.get_connection_info()) - .delete_auth_key(&auth_key.id.to_string()) + .delete_auth_key(&auth_key.key.to_string()) .await; assert_ok(response).await; @@ -777,7 +777,7 @@ mod tracker_apis { force_database_error(&api_server.tracker); let response = Client::new(api_server.get_connection_info()) - .delete_auth_key(&auth_key.id.to_string()) + .delete_auth_key(&auth_key.key.to_string()) .await; assert_failed_to_delete_key(response).await; @@ -797,7 +797,7 @@ mod tracker_apis { .unwrap(); let response = Client::new(connection_with_invalid_token(&api_server.get_bind_address())) - .delete_auth_key(&auth_key.id.to_string()) + .delete_auth_key(&auth_key.key.to_string()) .await; assert_token_not_valid(response).await; @@ -810,7 +810,7 @@ mod tracker_apis { .unwrap(); let response = Client::new(connection_with_no_token(&api_server.get_bind_address())) - .delete_auth_key(&auth_key.id.to_string()) + .delete_auth_key(&auth_key.key.to_string()) .await; assert_unauthorized(response).await; From af038dedc0876bda2809505998502e2067ebdb89 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Sun, 5 Mar 2023 12:17:39 +0000 Subject: [PATCH 4/4] refactor: rename structs, attributes and variables with sufix KeyId or key_id to `Key` and `key`. removing the `Id` and `id`, since the `KeyId` struct was renamed to `Key`. --- src/apis/handlers.rs | 6 ++--- src/databases/mod.rs | 4 ++-- .../axum_implementation/extractors/key.rs | 14 +++++------ .../axum_implementation/handlers/announce.rs | 6 ++--- src/http/axum_implementation/handlers/auth.rs | 4 ++-- .../axum_implementation/handlers/scrape.rs | 6 ++--- src/http/warp_implementation/filters.rs | 6 ++--- src/http/warp_implementation/handlers.rs | 12 +++++----- src/http/warp_implementation/routes.rs | 6 ++--- src/tracker/auth.rs | 18 +++++++------- src/tracker/error.rs | 4 ++-- src/tracker/mod.rs | 24 +++++++++---------- tests/http/client.rs | 18 +++++++------- tests/http/connection_info.rs | 4 ++-- tests/http_tracker.rs | 24 +++++++++---------- tests/tracker_api.rs | 10 ++++---- 16 files changed, 83 insertions(+), 83 deletions(-) diff --git a/src/apis/handlers.rs b/src/apis/handlers.rs index 652f491e5..410def39b 100644 --- a/src/apis/handlers.rs +++ b/src/apis/handlers.rs @@ -101,15 +101,15 @@ pub async fn generate_auth_key_handler(State(tracker): State>, Path } #[derive(Deserialize)] -pub struct KeyIdParam(String); +pub struct KeyParam(String); pub async fn delete_auth_key_handler( State(tracker): State>, - Path(seconds_valid_or_key): Path, + Path(seconds_valid_or_key): Path, ) -> Response { match Key::from_str(&seconds_valid_or_key.0) { Err(_) => invalid_auth_key_param_response(&seconds_valid_or_key.0), - Ok(key_id) => match tracker.remove_auth_key(&key_id.to_string()).await { + Ok(key) => match tracker.remove_auth_key(&key.to_string()).await { Ok(_) => ok_response(), Err(e) => failed_to_delete_key_response(e), }, diff --git a/src/databases/mod.rs b/src/databases/mod.rs index 038be0ea3..247f571d7 100644 --- a/src/databases/mod.rs +++ b/src/databases/mod.rs @@ -70,12 +70,12 @@ pub trait Database: Sync + Send { async fn remove_info_hash_from_whitelist(&self, info_hash: InfoHash) -> Result; - // todo: replace type `&str` with `&KeyId` + // todo: replace type `&str` with `&Key` async fn get_key_from_keys(&self, key: &str) -> Result, Error>; async fn add_key_to_keys(&self, auth_key: &auth::ExpiringKey) -> Result; - // todo: replace type `&str` with `&KeyId` + // todo: replace type `&str` with `&Key` async fn remove_key_from_keys(&self, key: &str) -> Result; async fn is_info_hash_whitelisted(&self, info_hash: &InfoHash) -> Result { diff --git a/src/http/axum_implementation/extractors/key.rs b/src/http/axum_implementation/extractors/key.rs index ecdc9d801..50aef4a7c 100644 --- a/src/http/axum_implementation/extractors/key.rs +++ b/src/http/axum_implementation/extractors/key.rs @@ -5,30 +5,30 @@ use axum::extract::{FromRequestParts, Path}; use axum::http::request::Parts; use axum::response::{IntoResponse, Response}; -use crate::http::axum_implementation::handlers::auth::{self, KeyIdParam}; +use crate::http::axum_implementation::handlers::auth::{self, KeyParam}; use crate::http::axum_implementation::responses; use crate::tracker::auth::Key; -pub struct ExtractKeyId(pub Key); +pub struct Extract(pub Key); #[async_trait] -impl FromRequestParts for ExtractKeyId +impl FromRequestParts for Extract where S: Send + Sync, { type Rejection = Response; async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { - match Path::::from_request_parts(parts, state).await { - Ok(key_id_param) => { - let Ok(key_id) = key_id_param.0.value().parse::() else { + match Path::::from_request_parts(parts, state).await { + Ok(key_param) => { + let Ok(key) = key_param.0.value().parse::() else { return Err(responses::error::Error::from( auth::Error::InvalidKeyFormat { location: Location::caller() }) .into_response()) }; - Ok(ExtractKeyId(key_id)) + Ok(Extract(key)) } Err(rejection) => match rejection { axum::extract::rejection::PathRejection::FailedToDeserializePathParams(_) => { diff --git a/src/http/axum_implementation/handlers/announce.rs b/src/http/axum_implementation/handlers/announce.rs index 93dbc8115..4bb06da73 100644 --- a/src/http/axum_implementation/handlers/announce.rs +++ b/src/http/axum_implementation/handlers/announce.rs @@ -8,7 +8,7 @@ use axum::response::{IntoResponse, Response}; use log::debug; use crate::http::axum_implementation::extractors::announce_request::ExtractRequest; -use crate::http::axum_implementation::extractors::key::ExtractKeyId; +use crate::http::axum_implementation::extractors::key::Extract; use crate::http::axum_implementation::extractors::peer_ip; use crate::http::axum_implementation::extractors::remote_client_ip::RemoteClientIp; use crate::http::axum_implementation::handlers::auth; @@ -41,12 +41,12 @@ pub async fn handle_without_key( pub async fn handle_with_key( State(tracker): State>, ExtractRequest(announce_request): ExtractRequest, - ExtractKeyId(key_id): ExtractKeyId, + Extract(key): Extract, remote_client_ip: RemoteClientIp, ) -> Response { debug!("http announce request: {:#?}", announce_request); - match tracker.authenticate(&key_id).await { + match tracker.authenticate(&key).await { Ok(_) => (), Err(error) => return responses::error::Error::from(error).into_response(), } diff --git a/src/http/axum_implementation/handlers/auth.rs b/src/http/axum_implementation/handlers/auth.rs index 5673ea851..b1b73e60e 100644 --- a/src/http/axum_implementation/handlers/auth.rs +++ b/src/http/axum_implementation/handlers/auth.rs @@ -7,9 +7,9 @@ use crate::http::axum_implementation::responses; use crate::tracker::auth; #[derive(Deserialize)] -pub struct KeyIdParam(String); +pub struct KeyParam(String); -impl KeyIdParam { +impl KeyParam { #[must_use] pub fn value(&self) -> String { self.0.clone() diff --git a/src/http/axum_implementation/handlers/scrape.rs b/src/http/axum_implementation/handlers/scrape.rs index 19d902f8e..41d6bf3dc 100644 --- a/src/http/axum_implementation/handlers/scrape.rs +++ b/src/http/axum_implementation/handlers/scrape.rs @@ -4,7 +4,7 @@ use axum::extract::State; use axum::response::{IntoResponse, Response}; use log::debug; -use crate::http::axum_implementation::extractors::key::ExtractKeyId; +use crate::http::axum_implementation::extractors::key::Extract; use crate::http::axum_implementation::extractors::peer_ip; use crate::http::axum_implementation::extractors::remote_client_ip::RemoteClientIp; use crate::http::axum_implementation::extractors::scrape_request::ExtractRequest; @@ -31,12 +31,12 @@ pub async fn handle_without_key( pub async fn handle_with_key( State(tracker): State>, ExtractRequest(scrape_request): ExtractRequest, - ExtractKeyId(key_id): ExtractKeyId, + Extract(key): Extract, remote_client_ip: RemoteClientIp, ) -> Response { debug!("http scrape request: {:#?}", &scrape_request); - match tracker.authenticate(&key_id).await { + match tracker.authenticate(&key).await { Ok(_) => (), Err(_) => return handle_fake_scrape(&tracker, &scrape_request, &remote_client_ip).await, } diff --git a/src/http/warp_implementation/filters.rs b/src/http/warp_implementation/filters.rs index a3000bfaa..06168d82a 100644 --- a/src/http/warp_implementation/filters.rs +++ b/src/http/warp_implementation/filters.rs @@ -37,11 +37,11 @@ pub fn with_peer_id() -> impl Filter + /// Pass Arc along #[must_use] -pub fn with_auth_key_id() -> impl Filter,), Error = Infallible> + Clone { +pub fn with_auth_key() -> impl Filter,), Error = Infallible> + Clone { warp::path::param::() .map(|key: String| { - let key_id = Key::from_str(&key); - match key_id { + let key = Key::from_str(&key); + match key { Ok(id) => Some(id), Err(_) => None, } diff --git a/src/http/warp_implementation/handlers.rs b/src/http/warp_implementation/handlers.rs index 4a64259bb..f9aedeb8f 100644 --- a/src/http/warp_implementation/handlers.rs +++ b/src/http/warp_implementation/handlers.rs @@ -22,11 +22,11 @@ use crate::tracker::{self, auth, peer, statistics, torrent}; /// Will return `ServerError` that wraps the `tracker::error::Error` if unable to `authenticate_request`. pub async fn authenticate( info_hash: &InfoHash, - auth_key_id: &Option, + auth_key: &Option, tracker: Arc, ) -> Result<(), Error> { tracker - .authenticate_request(info_hash, auth_key_id) + .authenticate_request(info_hash, auth_key) .await .map_err(|e| Error::TrackerError { source: (Arc::new(e) as Arc).into(), @@ -38,7 +38,7 @@ pub async fn authenticate( /// Will return `warp::Rejection` that wraps the `ServerError` if unable to `send_announce_response`. pub async fn handle_announce( announce_request: request::Announce, - auth_key_id: Option, + auth_key: Option, tracker: Arc, ) -> WebResult { debug!("http announce request: {:#?}", announce_request); @@ -46,7 +46,7 @@ pub async fn handle_announce( let info_hash = announce_request.info_hash; let remote_client_ip = announce_request.peer_addr; - authenticate(&info_hash, &auth_key_id, tracker.clone()).await?; + authenticate(&info_hash, &auth_key, tracker.clone()).await?; let mut peer = peer_builder::from_request(&announce_request, &remote_client_ip); @@ -78,7 +78,7 @@ pub async fn handle_announce( /// Will return `warp::Rejection` that wraps the `ServerError` if unable to `send_scrape_response`. pub async fn handle_scrape( scrape_request: request::Scrape, - auth_key_id: Option, + auth_key: Option, tracker: Arc, ) -> WebResult { let mut files: HashMap = HashMap::new(); @@ -87,7 +87,7 @@ pub async fn handle_scrape( for info_hash in &scrape_request.info_hashes { let scrape_entry = match db.get(info_hash) { Some(torrent_info) => { - if authenticate(info_hash, &auth_key_id, tracker.clone()).await.is_ok() { + if authenticate(info_hash, &auth_key, tracker.clone()).await.is_ok() { let (seeders, completed, leechers) = torrent_info.get_stats(); response::ScrapeEntry { complete: seeders, diff --git a/src/http/warp_implementation/routes.rs b/src/http/warp_implementation/routes.rs index 2ee60e8c9..c46c502e4 100644 --- a/src/http/warp_implementation/routes.rs +++ b/src/http/warp_implementation/routes.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use warp::{Filter, Rejection}; -use super::filters::{with_announce_request, with_auth_key_id, with_scrape_request, with_tracker}; +use super::filters::{with_announce_request, with_auth_key, with_scrape_request, with_tracker}; use super::handlers::{handle_announce, handle_scrape, send_error}; use crate::tracker; @@ -20,7 +20,7 @@ fn announce(tracker: Arc) -> impl Filter) -> impl Filter Result { if s.len() != AUTH_KEY_LENGTH { - return Err(ParseKeyIdError); + return Err(ParseKeyError); } Ok(Self(s.to_string())) @@ -106,10 +106,10 @@ pub enum Error { KeyVerificationError { source: LocatedError<'static, dyn std::error::Error + Send + Sync>, }, - #[error("Failed to read key: {key_id}, {location}")] + #[error("Failed to read key: {key}, {location}")] UnableToReadKey { location: &'static Location<'static>, - key_id: Box, + key: Box, }, #[error("Key has expired, {location}")] KeyExpired { location: &'static Location<'static> }, @@ -132,12 +132,12 @@ mod tests { use crate::tracker::auth; #[test] - fn auth_key_id_from_string() { + fn auth_key_from_string() { let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; - let auth_key_id = auth::Key::from_str(key_string); + let auth_key = auth::Key::from_str(key_string); - assert!(auth_key_id.is_ok()); - assert_eq!(auth_key_id.unwrap().to_string(), key_string); + assert!(auth_key.is_ok()); + assert_eq!(auth_key.unwrap().to_string(), key_string); } #[test] diff --git a/src/tracker/error.rs b/src/tracker/error.rs index f03f4b3e5..51bcbf3bb 100644 --- a/src/tracker/error.rs +++ b/src/tracker/error.rs @@ -4,9 +4,9 @@ use crate::located_error::LocatedError; #[derive(thiserror::Error, Debug, Clone)] pub enum Error { - #[error("The supplied key: {key_id:?}, is not valid: {source}")] + #[error("The supplied key: {key:?}, is not valid: {source}")] PeerKeyNotValid { - key_id: super::auth::Key, + key: super::auth::Key, source: LocatedError<'static, dyn std::error::Error + Send + Sync>, }, #[error("The peer is not authenticated, {location}")] diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index 34977b4de..2ebc4bfc3 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -200,9 +200,9 @@ impl Tracker { /// /// # Panics /// - /// Will panic if key cannot be converted into a valid `KeyId`. + /// Will panic if key cannot be converted into a valid `Key`. pub async fn remove_auth_key(&self, key: &str) -> Result<(), databases::error::Error> { - // todo: change argument `key: &str` to `key_id: &KeyId` + // todo: change argument `key: &str` to `key: &Key` self.database.remove_key_from_keys(key).await?; self.keys.write().await.remove(&key.parse::().unwrap()); Ok(()) @@ -211,13 +211,13 @@ impl Tracker { /// # Errors /// /// Will return a `key::Error` if unable to get any `auth_key`. - pub async fn verify_auth_key(&self, key_id: &Key) -> Result<(), auth::Error> { + pub async fn verify_auth_key(&self, key: &Key) -> Result<(), auth::Error> { // code-review: this function is public only because it's used in a test. // We should change the test and make it private. - match self.keys.read().await.get(key_id) { + match self.keys.read().await.get(key) { None => Err(auth::Error::UnableToReadKey { location: Location::caller(), - key_id: Box::new(key_id.clone()), + key: Box::new(key.clone()), }), Some(key) => auth::verify(key), } @@ -342,7 +342,7 @@ impl Tracker { Some(key) => { if let Err(e) = self.verify_auth_key(key).await { return Err(Error::PeerKeyNotValid { - key_id: key.clone(), + key: key.clone(), source: (Arc::new(e) as Arc).into(), }); } @@ -371,9 +371,9 @@ impl Tracker { /// # Errors /// /// Will return an error if the the authentication key cannot be verified. - pub async fn authenticate(&self, key_id: &Key) -> Result<(), auth::Error> { + pub async fn authenticate(&self, key: &Key) -> Result<(), auth::Error> { if self.is_private() { - self.verify_auth_key(key_id).await + self.verify_auth_key(key).await } else { Ok(()) } @@ -1165,9 +1165,9 @@ mod tests { async fn it_should_fail_authenticating_a_peer_when_it_uses_an_unregistered_key() { let tracker = private_tracker(); - let unregistered_key_id = auth::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + let unregistered_key = auth::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); - let result = tracker.authenticate(&unregistered_key_id).await; + let result = tracker.authenticate(&unregistered_key).await; assert!(result.is_err()); } @@ -1187,9 +1187,9 @@ mod tests { async fn it_should_fail_verifying_an_unregistered_authentication_key() { let tracker = private_tracker(); - let unregistered_key_id = auth::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + let unregistered_key = auth::Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); - assert!(tracker.verify_auth_key(&unregistered_key_id).await.is_err()); + assert!(tracker.verify_auth_key(&unregistered_key).await.is_err()); } #[tokio::test] diff --git a/tests/http/client.rs b/tests/http/client.rs index fa5fd5d16..762401078 100644 --- a/tests/http/client.rs +++ b/tests/http/client.rs @@ -11,7 +11,7 @@ use super::requests::scrape; pub struct Client { connection_info: ConnectionInfo, reqwest_client: ReqwestClient, - key_id: Option, + key: Option, } /// URL components in this context: @@ -27,7 +27,7 @@ impl Client { Self { connection_info, reqwest_client: reqwest::Client::builder().build().unwrap(), - key_id: None, + key: None, } } @@ -36,15 +36,15 @@ impl Client { Self { connection_info, reqwest_client: reqwest::Client::builder().local_address(local_address).build().unwrap(), - key_id: None, + key: None, } } - pub fn authenticated(connection_info: ConnectionInfo, key_id: Key) -> Self { + pub fn authenticated(connection_info: ConnectionInfo, key: Key) -> Self { Self { connection_info, reqwest_client: reqwest::Client::builder().build().unwrap(), - key_id: Some(key_id), + key: Some(key), } } @@ -56,8 +56,8 @@ impl Client { self.get(&self.build_scrape_path_and_query(query)).await } - pub async fn announce_with_header(&self, query: &Query, key_id: &str, value: &str) -> Response { - self.get_with_header(&self.build_announce_path_and_query(query), key_id, value) + pub async fn announce_with_header(&self, query: &Query, key: &str, value: &str) -> Response { + self.get_with_header(&self.build_announce_path_and_query(query), key, value) .await } @@ -83,8 +83,8 @@ impl Client { } fn build_path(&self, path: &str) -> String { - match &self.key_id { - Some(key_id) => format!("{path}/{key_id}"), + match &self.key { + Some(key) => format!("{path}/{key}"), None => path.to_string(), } } diff --git a/tests/http/connection_info.rs b/tests/http/connection_info.rs index eedaa73f0..5736271fd 100644 --- a/tests/http/connection_info.rs +++ b/tests/http/connection_info.rs @@ -3,14 +3,14 @@ use torrust_tracker::tracker::auth::Key; #[derive(Clone, Debug)] pub struct ConnectionInfo { pub bind_address: String, - pub key_id: Option, + pub key: Option, } impl ConnectionInfo { pub fn anonymous(bind_address: &str) -> Self { Self { bind_address: bind_address.to_string(), - key_id: None, + key: None, } } } diff --git a/tests/http_tracker.rs b/tests/http_tracker.rs index b1b90f923..4219be30a 100644 --- a/tests/http_tracker.rs +++ b/tests/http_tracker.rs @@ -1128,9 +1128,9 @@ mod warp_http_tracker_server { let http_tracker_server = start_private_http_tracker(Version::Warp).await; // The tracker does not have this key - let unregistered_key_id = Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + let unregistered_key = Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); - let response = Client::authenticated(http_tracker_server.get_connection_info(), unregistered_key_id) + let response = Client::authenticated(http_tracker_server.get_connection_info(), unregistered_key) .announce(&QueryBuilder::default().query()) .await; @@ -1242,9 +1242,9 @@ mod warp_http_tracker_server { ) .await; - let false_key_id: Key = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".parse().unwrap(); + let false_key: Key = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".parse().unwrap(); - let response = Client::authenticated(http_tracker.get_connection_info(), false_key_id) + let response = Client::authenticated(http_tracker.get_connection_info(), false_key) .scrape( &requests::scrape::QueryBuilder::default() .with_one_info_hash(&info_hash) @@ -2437,11 +2437,11 @@ mod axum_http_tracker_server { async fn should_fail_if_the_key_query_param_cannot_be_parsed() { let http_tracker_server = start_private_http_tracker(Version::Axum).await; - let invalid_key_id = "INVALID_KEY_ID"; + let invalid_key = "INVALID_KEY"; let response = Client::new(http_tracker_server.get_connection_info()) .get(&format!( - "announce/{invalid_key_id}?info_hash=%81%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00&peer_addr=2.137.87.41&downloaded=0&uploaded=0&peer_id=-qB00000000000000001&port=17548&left=0&event=completed&compact=0" + "announce/{invalid_key}?info_hash=%81%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00%00&peer_addr=2.137.87.41&downloaded=0&uploaded=0&peer_id=-qB00000000000000001&port=17548&left=0&event=completed&compact=0" )) .await; @@ -2453,9 +2453,9 @@ mod axum_http_tracker_server { let http_tracker_server = start_private_http_tracker(Version::Axum).await; // The tracker does not have this key - let unregistered_key_id = Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); + let unregistered_key = Key::from_str("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(); - let response = Client::authenticated(http_tracker_server.get_connection_info(), unregistered_key_id) + let response = Client::authenticated(http_tracker_server.get_connection_info(), unregistered_key) .announce(&QueryBuilder::default().query()) .await; @@ -2484,11 +2484,11 @@ mod axum_http_tracker_server { async fn should_fail_if_the_key_query_param_cannot_be_parsed() { let http_tracker_server = start_private_http_tracker(Version::Axum).await; - let invalid_key_id = "INVALID_KEY_ID"; + let invalid_key = "INVALID_KEY"; let response = Client::new(http_tracker_server.get_connection_info()) .get(&format!( - "scrape/{invalid_key_id}?info_hash=%3B%24U%04%CF%5F%11%BB%DB%E1%20%1C%EAjk%F4Z%EE%1B%C0" + "scrape/{invalid_key}?info_hash=%3B%24U%04%CF%5F%11%BB%DB%E1%20%1C%EAjk%F4Z%EE%1B%C0" )) .await; @@ -2583,9 +2583,9 @@ mod axum_http_tracker_server { ) .await; - let false_key_id: Key = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".parse().unwrap(); + let false_key: Key = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ".parse().unwrap(); - let response = Client::authenticated(http_tracker.get_connection_info(), false_key_id) + let response = Client::authenticated(http_tracker.get_connection_info(), false_key) .scrape( &requests::scrape::QueryBuilder::default() .with_one_info_hash(&info_hash) diff --git a/tests/tracker_api.rs b/tests/tracker_api.rs index 600d26f2f..35d9af248 100644 --- a/tests/tracker_api.rs +++ b/tests/tracker_api.rs @@ -741,10 +741,10 @@ mod tracker_apis { } #[tokio::test] - async fn should_fail_deleting_an_auth_key_when_the_key_id_is_invalid() { + async fn should_fail_deleting_an_auth_key_when_the_key_is_invalid() { let api_server = start_default_api().await; - let invalid_auth_key_ids = [ + let invalid_auth_keys = [ // "", it returns a 404 // " ", it returns a 404 "0", @@ -754,12 +754,12 @@ mod tracker_apis { "IrweYtVuQPGbG9Jzx1DihcPmJGGpVy8zs", // 34 char key cspell:disable-line ]; - for invalid_auth_key_id in &invalid_auth_key_ids { + for invalid_auth_key in &invalid_auth_keys { let response = Client::new(api_server.get_connection_info()) - .delete_auth_key(invalid_auth_key_id) + .delete_auth_key(invalid_auth_key) .await; - assert_invalid_auth_key_param(response, invalid_auth_key_id).await; + assert_invalid_auth_key_param(response, invalid_auth_key).await; } }