Skip to content

Commit

Permalink
fix: [#1257] bug 3. Expiring auth keys ignore fractions of seconds
Browse files Browse the repository at this point in the history
The `Duration` of a peer Key can have gractions of seconds. However we
only store seconds (integer) in the database.

When comparing peer keys we should ignore the fractions.
  • Loading branch information
josecelano committed Feb 10, 2025
1 parent b94179d commit 613efb2
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 17 deletions.
23 changes: 21 additions & 2 deletions packages/tracker-core/src/authentication/key/peer_key.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::str::FromStr;
use std::time::Duration;

use derive_more::Display;
use rand::distr::Alphanumeric;
Expand All @@ -12,7 +13,7 @@ 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)]
#[derive(Serialize, Deserialize, Debug, Clone)]
pub struct PeerKey {
/// Random 32-char string. For example: `YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ`
pub key: Key,
Expand All @@ -22,6 +23,21 @@ pub struct PeerKey {
pub valid_until: Option<DurationSinceUnixEpoch>,
}

impl PartialEq for PeerKey {
fn eq(&self, other: &Self) -> bool {
// We ignore the fractions of seconds when comparing the timestamps
// because we only store the seconds in the database.
self.key == other.key
&& match (&self.valid_until, &other.valid_until) {
(Some(a), Some(b)) => a.as_secs() == b.as_secs(),
(None, None) => true,
_ => false,
}
}
}

impl Eq for PeerKey {}

impl std::fmt::Display for PeerKey {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self.expiry_time() {
Expand All @@ -47,7 +63,10 @@ impl PeerKey {
/// (this will naturally happen in 292.5 billion years)
#[must_use]
pub fn expiry_time(&self) -> Option<chrono::DateTime<chrono::Utc>> {
self.valid_until.map(convert_from_timestamp_to_datetime_utc)
// We remove the fractions of seconds because we only store the seconds
// in the database.
self.valid_until
.map(|valid_until| convert_from_timestamp_to_datetime_utc(Duration::from_secs(valid_until.as_secs())))
}
}

Expand Down
17 changes: 2 additions & 15 deletions packages/tracker-core/src/databases/driver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,21 +180,8 @@ mod tests {
// Get the key back
let stored_peer_key = driver.get_key_from_keys(&peer_key.key()).unwrap().unwrap();

/* todo:
The expiration time recovered from the database is not the same
as the one we set. It includes a small offset (nanoseconds).
left: PeerKey { key: Key("7HP1NslpuQn6kLVAgAF4nFpnZNSQ4hrx"), valid_until: Some(1739182308s) }
right: PeerKey { key: Key("7HP1NslpuQn6kLVAgAF4nFpnZNSQ4hrx"), valid_until: Some(1739182308.603691299s)
*/

assert_eq!(stored_peer_key.key(), peer_key.key());
assert_eq!(
stored_peer_key.valid_until.unwrap().as_secs(),
peer_key.valid_until.unwrap().as_secs()
);
assert_eq!(stored_peer_key, peer_key);
assert_eq!(stored_peer_key.expiry_time(), peer_key.expiry_time());
}

pub fn it_should_remove_a_permanent_authentication_key(driver: &Arc<Box<dyn Database>>) {
Expand Down

0 comments on commit 613efb2

Please sign in to comment.