Skip to content

Commit

Permalink
Merge #990: Overhaul tracker keys: add more tests
Browse files Browse the repository at this point in the history
349692b test: [#989] add more tests for keys (Jose Celano)
8d58882 refactor: make method private (Jose Celano)

Pull request description:

  There are four ways of adding keys to the tracker. One for each combination of:

  - Expiring or permanent key.
  - Pre-generated (uploaded) or randomly generated key.

  This PR adds new tests for each case.

ACKs for top commit:
  josecelano:
    ACK 349692b

Tree-SHA512: 16f133254000a9b2caaa25ba335447a98cee0c9763c700fc430788cf55b73cb102d47adbf0f329d21e7c584686312c2e1ae724af37fa00475d8f28c88cb270c0
  • Loading branch information
josecelano committed Aug 1, 2024
2 parents e00feef + 349692b commit d47ff21
Show file tree
Hide file tree
Showing 2 changed files with 191 additions and 66 deletions.
248 changes: 188 additions & 60 deletions src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -988,9 +988,7 @@ impl Tracker {
/// # Errors
///
/// Will return a `key::Error` if unable to get any `auth_key`.
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.
async fn verify_auth_key(&self, key: &Key) -> Result<(), auth::Error> {
match self.keys.read().await.get(key) {
None => Err(auth::Error::UnableToReadKey {
location: Location::caller(),
Expand Down Expand Up @@ -1787,35 +1785,8 @@ mod tests {
use std::str::FromStr;
use std::time::Duration;

use torrust_tracker_clock::clock::Time;
use torrust_tracker_configuration::v2::core::PrivateMode;

use crate::core::auth::{self, Key};
use crate::core::auth::{self};
use crate::core::tests::the_tracker::private_tracker;
use crate::CurrentClock;

#[tokio::test]
async fn it_should_generate_the_expiring_authentication_keys() {
let tracker = private_tracker();

let key = tracker.generate_auth_key(Some(Duration::from_secs(100))).await.unwrap();

assert_eq!(
key.valid_until,
Some(CurrentClock::now_add(&Duration::from_secs(100)).unwrap())
);
}

#[tokio::test]
async fn it_should_authenticate_a_peer_by_using_a_key() {
let tracker = private_tracker();

let expiring_key = tracker.generate_auth_key(Some(Duration::from_secs(100))).await.unwrap();

let result = tracker.authenticate(&expiring_key.key()).await;

assert!(result.is_ok());
}

#[tokio::test]
async fn it_should_fail_authenticating_a_peer_when_it_uses_an_unregistered_key() {
Expand All @@ -1828,35 +1799,6 @@ mod tests {
assert!(result.is_err());
}

#[tokio::test]
async fn it_should_verify_a_valid_authentication_key() {
// todo: this should not be tested directly because
// `verify_auth_key` should be a private method.
let tracker = private_tracker();

let expiring_key = tracker.generate_auth_key(Some(Duration::from_secs(100))).await.unwrap();

assert!(tracker.verify_auth_key(&expiring_key.key()).await.is_ok());
}

#[tokio::test]
async fn it_should_accept_an_expired_key_when_checking_expiration_is_disabled_in_configuration() {
let mut tracker = private_tracker();

tracker.config.private_mode = Some(PrivateMode {
check_keys_expiration: false,
});

let past_time = Some(Duration::ZERO);

let expiring_key = tracker
.add_auth_key(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), past_time)
.await
.unwrap();

assert!(tracker.authenticate(&expiring_key.key()).await.is_ok());
}

#[tokio::test]
async fn it_should_fail_verifying_an_unregistered_authentication_key() {
let tracker = private_tracker();
Expand Down Expand Up @@ -1892,6 +1834,192 @@ mod tests {
assert!(result.is_ok());
assert!(tracker.verify_auth_key(&expiring_key.key()).await.is_ok());
}

mod with_expiring_and {

mod randomly_generated_keys {
use std::time::Duration;

use torrust_tracker_clock::clock::Time;
use torrust_tracker_configuration::v2::core::PrivateMode;

use crate::core::auth::Key;
use crate::core::tests::the_tracker::private_tracker;
use crate::CurrentClock;

#[tokio::test]
async fn it_should_generate_the_key() {
let tracker = private_tracker();

let peer_key = tracker.generate_auth_key(Some(Duration::from_secs(100))).await.unwrap();

assert_eq!(
peer_key.valid_until,
Some(CurrentClock::now_add(&Duration::from_secs(100)).unwrap())
);
}

#[tokio::test]
async fn it_should_authenticate_a_peer_with_the_key() {
let tracker = private_tracker();

let peer_key = tracker.generate_auth_key(Some(Duration::from_secs(100))).await.unwrap();

let result = tracker.authenticate(&peer_key.key()).await;

assert!(result.is_ok());
}

#[tokio::test]
async fn it_should_accept_an_expired_key_when_checking_expiration_is_disabled_in_configuration() {
let mut tracker = private_tracker();

tracker.config.private_mode = Some(PrivateMode {
check_keys_expiration: false,
});

let past_timestamp = Duration::ZERO;

let peer_key = tracker
.add_auth_key(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap(), Some(past_timestamp))
.await
.unwrap();

assert!(tracker.authenticate(&peer_key.key()).await.is_ok());
}
}

mod pre_generated_keys {
use std::time::Duration;

use torrust_tracker_clock::clock::Time;
use torrust_tracker_configuration::v2::core::PrivateMode;

use crate::core::auth::Key;
use crate::core::tests::the_tracker::private_tracker;
use crate::core::AddKeyRequest;
use crate::CurrentClock;

#[tokio::test]
async fn it_should_add_a_pre_generated_key() {
let tracker = private_tracker();

let peer_key = tracker
.add_peer_key(AddKeyRequest {
opt_key: Some(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap().to_string()),
opt_seconds_valid: Some(100),
})
.await
.unwrap();

assert_eq!(
peer_key.valid_until,
Some(CurrentClock::now_add(&Duration::from_secs(100)).unwrap())
);
}

#[tokio::test]
async fn it_should_authenticate_a_peer_with_the_key() {
let tracker = private_tracker();

let peer_key = tracker
.add_peer_key(AddKeyRequest {
opt_key: Some(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap().to_string()),
opt_seconds_valid: Some(100),
})
.await
.unwrap();

let result = tracker.authenticate(&peer_key.key()).await;

assert!(result.is_ok());
}

#[tokio::test]
async fn it_should_accept_an_expired_key_when_checking_expiration_is_disabled_in_configuration() {
let mut tracker = private_tracker();

tracker.config.private_mode = Some(PrivateMode {
check_keys_expiration: false,
});

let peer_key = tracker
.add_peer_key(AddKeyRequest {
opt_key: Some(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap().to_string()),
opt_seconds_valid: Some(0),
})
.await
.unwrap();

assert!(tracker.authenticate(&peer_key.key()).await.is_ok());
}
}
}

mod with_permanent_and {

mod randomly_generated_keys {
use crate::core::tests::the_tracker::private_tracker;

#[tokio::test]
async fn it_should_generate_the_key() {
let tracker = private_tracker();

let peer_key = tracker.generate_permanent_auth_key().await.unwrap();

assert_eq!(peer_key.valid_until, None);
}

#[tokio::test]
async fn it_should_authenticate_a_peer_with_the_key() {
let tracker = private_tracker();

let peer_key = tracker.generate_permanent_auth_key().await.unwrap();

let result = tracker.authenticate(&peer_key.key()).await;

assert!(result.is_ok());
}
}

mod pre_generated_keys {
use crate::core::auth::Key;
use crate::core::tests::the_tracker::private_tracker;
use crate::core::AddKeyRequest;

#[tokio::test]
async fn it_should_add_a_pre_generated_key() {
let tracker = private_tracker();

let peer_key = tracker
.add_peer_key(AddKeyRequest {
opt_key: Some(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap().to_string()),
opt_seconds_valid: None,
})
.await
.unwrap();

assert_eq!(peer_key.valid_until, None);
}

#[tokio::test]
async fn it_should_authenticate_a_peer_with_the_key() {
let tracker = private_tracker();

let peer_key = tracker
.add_peer_key(AddKeyRequest {
opt_key: Some(Key::new("YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ").unwrap().to_string()),
opt_seconds_valid: None,
})
.await
.unwrap();

let result = tracker.authenticate(&peer_key.key()).await;

assert!(result.is_ok());
}
}
}
}

mod handling_an_announce_request {}
Expand Down
9 changes: 3 additions & 6 deletions tests/servers/api/v1/contract/context/auth_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,9 @@ async fn should_allow_generating_a_new_random_auth_key() {

let auth_key_resource = assert_auth_key_utf8(response).await;

// Verify the key with the tracker
assert!(env
.tracker
.verify_auth_key(&auth_key_resource.key.parse::<Key>().unwrap())
.authenticate(&auth_key_resource.key.parse::<Key>().unwrap())
.await
.is_ok());

Expand All @@ -49,10 +48,9 @@ async fn should_allow_uploading_a_preexisting_auth_key() {

let auth_key_resource = assert_auth_key_utf8(response).await;

// Verify the key with the tracker
assert!(env
.tracker
.verify_auth_key(&auth_key_resource.key.parse::<Key>().unwrap())
.authenticate(&auth_key_resource.key.parse::<Key>().unwrap())
.await
.is_ok());

Expand Down Expand Up @@ -357,10 +355,9 @@ mod deprecated_generate_key_endpoint {

let auth_key_resource = assert_auth_key_utf8(response).await;

// Verify the key with the tracker
assert!(env
.tracker
.verify_auth_key(&auth_key_resource.key.parse::<Key>().unwrap())
.authenticate(&auth_key_resource.key.parse::<Key>().unwrap())
.await
.is_ok());

Expand Down

0 comments on commit d47ff21

Please sign in to comment.