From 084b2acfb378c5df795644ea20a7adf30dd0c2d6 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 14 Mar 2023 13:28:27 +0000 Subject: [PATCH] feat(api): [#120] use datetime ISO 8601 in auth key enpoint instead of timestamp. --- src/apis/v1/context/auth_key/resources.rs | 53 ++++++---- src/protocol/clock/mod.rs | 70 +++++++++++++ src/tracker/auth.rs | 117 ++++++++++++++-------- src/tracker/mod.rs | 18 ++-- tests/servers/http/v1/contract.rs | 8 +- 5 files changed, 190 insertions(+), 76 deletions(-) diff --git a/src/apis/v1/context/auth_key/resources.rs b/src/apis/v1/context/auth_key/resources.rs index 72ef32a95..cf43a6f3d 100644 --- a/src/apis/v1/context/auth_key/resources.rs +++ b/src/apis/v1/context/auth_key/resources.rs @@ -2,25 +2,21 @@ use std::convert::From; use serde::{Deserialize, Serialize}; -use crate::protocol::clock::DurationSinceUnixEpoch; +use crate::protocol::clock::convert_from_iso_8601_to_timestamp; 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 valid_until: Option, // todo: `auth::ExpiringKey` has now always a value (API breaking change!) + pub key: String, + pub valid_until: u64, // todo: remove when the torrust-index-backend starts using the `expiry_time` attribute. + pub expiry_time: String, } 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 { key: auth_key_resource.key.parse::().unwrap(), - valid_until, + valid_until: convert_from_iso_8601_to_timestamp(&auth_key_resource.expiry_time), } } } @@ -29,7 +25,8 @@ impl From for AuthKey { fn from(auth_key: auth::ExpiringKey) -> Self { AuthKey { key: auth_key.key.to_string(), - valid_until: Some(auth_key.valid_until.as_secs()), + valid_until: auth_key.valid_until.as_secs(), + expiry_time: auth_key.expiry_time().to_string(), } } } @@ -42,38 +39,53 @@ mod tests { use crate::protocol::clock::{Current, TimeNow}; use crate::tracker::auth::{self, Key}; + struct TestTime { + pub timestamp: u64, + pub iso_8601_v1: String, + pub iso_8601_v2: String, + } + + fn one_hour_after_unix_epoch() -> TestTime { + let timestamp = 60_u64; + let iso_8601_v1 = "1970-01-01T00:01:00.000Z".to_string(); + let iso_8601_v2 = "1970-01-01 00:01:00 UTC".to_string(); + TestTime { + timestamp, + iso_8601_v1, + iso_8601_v2, + } + } + #[test] fn it_should_be_convertible_into_an_auth_key() { - let duration_in_secs = 60; - let auth_key_resource = AuthKey { key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".to_string(), // cspell:disable-line - valid_until: Some(duration_in_secs), + valid_until: one_hour_after_unix_epoch().timestamp, + expiry_time: one_hour_after_unix_epoch().iso_8601_v1, }; assert_eq!( auth::ExpiringKey::from(auth_key_resource), auth::ExpiringKey { key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line - valid_until: Current::add(&Duration::new(duration_in_secs, 0)).unwrap() + valid_until: Current::add(&Duration::new(one_hour_after_unix_epoch().timestamp, 0)).unwrap() } ); } #[test] fn it_should_be_convertible_from_an_auth_key() { - let duration_in_secs = 60; - let auth_key = auth::ExpiringKey { key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".parse::().unwrap(), // cspell:disable-line - valid_until: Current::add(&Duration::new(duration_in_secs, 0)).unwrap(), + valid_until: Current::add(&Duration::new(one_hour_after_unix_epoch().timestamp, 0)).unwrap(), }; assert_eq!( AuthKey::from(auth_key), AuthKey { key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".to_string(), // cspell:disable-line - valid_until: Some(duration_in_secs) + valid_until: one_hour_after_unix_epoch().timestamp, + expiry_time: one_hour_after_unix_epoch().iso_8601_v2, } ); } @@ -83,10 +95,11 @@ mod tests { assert_eq!( serde_json::to_string(&AuthKey { key: "IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM".to_string(), // cspell:disable-line - valid_until: Some(60) + valid_until: one_hour_after_unix_epoch().timestamp, + expiry_time: one_hour_after_unix_epoch().iso_8601_v1, }) .unwrap(), - "{\"key\":\"IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM\",\"valid_until\":60}" // cspell:disable-line + "{\"key\":\"IaWDneuFNZi8IB4MPA3qW1CD0M30EZSM\",\"valid_until\":60,\"expiry_time\":\"1970-01-01T00:01:00.000Z\"}" // cspell:disable-line ); } } diff --git a/src/protocol/clock/mod.rs b/src/protocol/clock/mod.rs index 7868d4c5e..73df37b58 100644 --- a/src/protocol/clock/mod.rs +++ b/src/protocol/clock/mod.rs @@ -1,6 +1,9 @@ use std::num::IntErrorKind; +use std::str::FromStr; use std::time::Duration; +use chrono::{DateTime, NaiveDateTime, Utc}; + pub type DurationSinceUnixEpoch = Duration; #[derive(Debug)] @@ -36,6 +39,40 @@ pub trait TimeNow: Time { } } +/// # Panics +/// +/// Will panic if the input time cannot be converted to `DateTime::`. +/// +#[must_use] +pub fn convert_from_iso_8601_to_timestamp(iso_8601: &str) -> DurationSinceUnixEpoch { + convert_from_datetime_utc_to_timestamp(&DateTime::::from_str(iso_8601).unwrap()) +} + +/// # Panics +/// +/// Will panic if the input time overflows the u64 type. +/// +#[must_use] +pub fn convert_from_datetime_utc_to_timestamp(datetime_utc: &DateTime) -> DurationSinceUnixEpoch { + DurationSinceUnixEpoch::from_secs(u64::try_from(datetime_utc.timestamp()).expect("Overflow of u64 seconds, very future!")) +} + +/// # Panics +/// +/// Will panic if the input time overflows the i64 type. +/// +#[must_use] +pub fn convert_from_timestamp_to_datetime_utc(duration: DurationSinceUnixEpoch) -> DateTime { + DateTime::::from_utc( + NaiveDateTime::from_timestamp_opt( + i64::try_from(duration.as_secs()).expect("Overflow of i64 seconds, very future!"), + duration.subsec_nanos(), + ) + .unwrap(), + Utc, + ) +} + #[cfg(test)] mod tests { use std::any::TypeId; @@ -54,6 +91,39 @@ mod tests { assert_ne!(TypeId::of::(), TypeId::of::()); assert_ne!(Stopped::now(), Working::now()); } + + mod timestamp { + use chrono::{DateTime, NaiveDateTime, Utc}; + + use crate::protocol::clock::{ + convert_from_datetime_utc_to_timestamp, convert_from_iso_8601_to_timestamp, convert_from_timestamp_to_datetime_utc, + DurationSinceUnixEpoch, + }; + + #[test] + fn should_be_converted_to_datetime_utc() { + let timestamp = DurationSinceUnixEpoch::ZERO; + assert_eq!( + convert_from_timestamp_to_datetime_utc(timestamp), + DateTime::::from_utc(NaiveDateTime::from_timestamp_opt(0, 0).unwrap(), Utc) + ); + } + + #[test] + fn should_be_converted_from_datetime_utc() { + let datetime = DateTime::::from_utc(NaiveDateTime::from_timestamp_opt(0, 0).unwrap(), Utc); + assert_eq!( + convert_from_datetime_utc_to_timestamp(&datetime), + DurationSinceUnixEpoch::ZERO + ); + } + + #[test] + fn should_be_converted_from_datetime_utc_in_iso_8601() { + let iso_8601 = "1970-01-01T00:00:00.000Z".to_string(); + assert_eq!(convert_from_iso_8601_to_timestamp(&iso_8601), DurationSinceUnixEpoch::ZERO); + } + } } mod working_clock { diff --git a/src/tracker/auth.rs b/src/tracker/auth.rs index e3c12a828..75bc543a8 100644 --- a/src/tracker/auth.rs +++ b/src/tracker/auth.rs @@ -3,7 +3,6 @@ use std::str::FromStr; use std::sync::Arc; use std::time::Duration; -use chrono::{DateTime, NaiveDateTime, Utc}; use derive_more::Display; use log::debug; use rand::distributions::Alphanumeric; @@ -12,7 +11,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use torrust_tracker_located_error::LocatedError; -use crate::protocol::clock::{Current, DurationSinceUnixEpoch, Time, TimeNow}; +use crate::protocol::clock::{convert_from_timestamp_to_datetime_utc, Current, DurationSinceUnixEpoch, Time, TimeNow}; use crate::protocol::common::AUTH_KEY_LENGTH; #[must_use] @@ -59,27 +58,28 @@ pub struct ExpiringKey { impl std::fmt::Display for ExpiringKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "key: `{}`, valid until `{}`", - self.key, - DateTime::::from_utc( - NaiveDateTime::from_timestamp_opt( - i64::try_from(self.valid_until.as_secs()).expect("Overflow of i64 seconds, very future!"), - self.valid_until.subsec_nanos(), - ) - .unwrap(), - Utc - ) - ) + write!(f, "key: `{}`, valid until `{}`", self.key, self.expiry_time()) } } impl ExpiringKey { #[must_use] - pub fn id(&self) -> Key { + 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 ui64 type. + /// + #[must_use] + pub fn expiry_time(&self) -> chrono::DateTime { + convert_from_timestamp_to_datetime_utc(self.valid_until) + } } #[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone, Display, Hash)] @@ -126,44 +126,75 @@ impl From for Error { #[cfg(test)] mod tests { - use std::str::FromStr; - use std::time::Duration; - use crate::protocol::clock::{Current, StoppedTime}; - use crate::tracker::auth; + mod key { + use std::str::FromStr; - #[test] - fn auth_key_from_string() { - let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; - let auth_key = auth::Key::from_str(key_string); + use crate::tracker::auth::Key; - assert!(auth_key.is_ok()); - assert_eq!(auth_key.unwrap().to_string(), key_string); + #[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 generate_valid_auth_key() { - let auth_key = auth::generate(Duration::new(9999, 0)); + mod expiring_auth_key { + use std::str::FromStr; + use std::time::Duration; - assert!(auth::verify(&auth_key).is_ok()); - } + use crate::protocol::clock::{Current, StoppedTime}; + use crate::tracker::auth; + + #[test] + fn should_be_parsed_from_an_string() { + let key_string = "YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ"; + let auth_key = auth::Key::from_str(key_string); - #[test] - fn generate_and_check_expired_auth_key() { - // Set the time to the current time. - Current::local_set_to_system_time_now(); + assert!(auth_key.is_ok()); + assert_eq!(auth_key.unwrap().to_string(), key_string); + } - // Make key that is valid for 19 seconds. - let auth_key = auth::generate(Duration::from_secs(19)); + #[test] + fn should_be_displayed() { + // Set the time to the current time. + Current::local_set_to_unix_epoch(); - // Mock the time has passed 10 sec. - Current::local_add(&Duration::from_secs(10)).unwrap(); + let expiring_key = auth::generate(Duration::from_secs(0)); - assert!(auth::verify(&auth_key).is_ok()); + assert_eq!( + expiring_key.to_string(), + format!("key: `{}`, valid until `1970-01-01 00:00:00 UTC`", expiring_key.key) // cspell:disable-line + ); + } - // Mock the time has passed another 10 sec. - Current::local_add(&Duration::from_secs(10)).unwrap(); + #[test] + fn should_be_generated_with_a_expiration_time() { + let expiring_key = auth::generate(Duration::new(9999, 0)); - assert!(auth::verify(&auth_key).is_err()); + assert!(auth::verify(&expiring_key).is_ok()); + } + + #[test] + fn should_be_generate_and_verified() { + // Set the time to the current time. + Current::local_set_to_system_time_now(); + + // Make key that is valid for 19 seconds. + let expiring_key = auth::generate(Duration::from_secs(19)); + + // Mock the time has passed 10 sec. + Current::local_add(&Duration::from_secs(10)).unwrap(); + + assert!(auth::verify(&expiring_key).is_ok()); + + // Mock the time has passed another 10 sec. + Current::local_add(&Duration::from_secs(10)).unwrap(); + + assert!(auth::verify(&expiring_key).is_err()); + } } } diff --git a/src/tracker/mod.rs b/src/tracker/mod.rs index 8a9739793..71bb41f90 100644 --- a/src/tracker/mod.rs +++ b/src/tracker/mod.rs @@ -1132,9 +1132,9 @@ mod tests { async fn it_should_authenticate_a_peer_by_using_a_key() { let tracker = private_tracker(); - let key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); + let expiring_key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); - let result = tracker.authenticate(&key.id()).await; + let result = tracker.authenticate(&expiring_key.key()).await; assert!(result.is_ok()); } @@ -1156,9 +1156,9 @@ mod tests { // `verify_auth_key` should be a private method. let tracker = private_tracker(); - let key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); + let expiring_key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); - assert!(tracker.verify_auth_key(&key.id()).await.is_ok()); + assert!(tracker.verify_auth_key(&expiring_key.key()).await.is_ok()); } #[tokio::test] @@ -1176,25 +1176,25 @@ mod tests { let expiring_key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); - let result = tracker.remove_auth_key(&expiring_key.id()).await; + let result = tracker.remove_auth_key(&expiring_key.key()).await; assert!(result.is_ok()); - assert!(tracker.verify_auth_key(&expiring_key.id()).await.is_err()); + assert!(tracker.verify_auth_key(&expiring_key.key()).await.is_err()); } #[tokio::test] async fn it_should_load_authentication_keys_from_the_database() { let tracker = private_tracker(); - let key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); + let expiring_key = tracker.generate_auth_key(Duration::from_secs(100)).await.unwrap(); // Remove the newly generated key in memory - tracker.keys.write().await.remove(&key.id()); + tracker.keys.write().await.remove(&expiring_key.key()); let result = tracker.load_keys_from_database().await; assert!(result.is_ok()); - assert!(tracker.verify_auth_key(&key.id()).await.is_ok()); + assert!(tracker.verify_auth_key(&expiring_key.key()).await.is_ok()); } } diff --git a/tests/servers/http/v1/contract.rs b/tests/servers/http/v1/contract.rs index eda42f1ee..501c0f6fa 100644 --- a/tests/servers/http/v1/contract.rs +++ b/tests/servers/http/v1/contract.rs @@ -1215,9 +1215,9 @@ mod configured_as_private { async fn should_respond_to_authenticated_peers() { let test_env = running_test_environment::(configuration::ephemeral_mode_private()).await; - let key = test_env.tracker.generate_auth_key(Duration::from_secs(60)).await.unwrap(); + let expiring_key = test_env.tracker.generate_auth_key(Duration::from_secs(60)).await.unwrap(); - let response = Client::authenticated(*test_env.bind_address(), key.id()) + let response = Client::authenticated(*test_env.bind_address(), expiring_key.key()) .announce(&QueryBuilder::default().query()) .await; @@ -1353,9 +1353,9 @@ mod configured_as_private { ) .await; - let key = test_env.tracker.generate_auth_key(Duration::from_secs(60)).await.unwrap(); + let expiring_key = test_env.tracker.generate_auth_key(Duration::from_secs(60)).await.unwrap(); - let response = Client::authenticated(*test_env.bind_address(), key.id()) + let response = Client::authenticated(*test_env.bind_address(), expiring_key.key()) .scrape( &requests::scrape::QueryBuilder::default() .with_one_info_hash(&info_hash)