Merge #494: Overhaul tracker keys in torrents and magnet links

9b7c5c8 feat: [#488] panic starting the app when tracker config is invalid (Jose Celano)
36e46fe chore(deps): add cargo dependency url to parse URLs (Jose Celano)
b8624a8 feat!: [#488] return 503 instead of 500 when tracker is offline (Jose Celano)
d100b5b fix: [#488] broken test after removing duplicate tracker urls from magnet links (Jose Celano)
c1fd866 feat: [#488] return an error in the torrent details endpoint when user tracker keys can't be generated (Jose Celano)
b41488b fix: [#488] torrent without tracker keys for open tracker (Jose Celano)

Pull request description:

  ```
  1. Tracker running in `Public` or `Whitelisted` mode.
     Downloaded torrents should never include any tracker key.
  3. Tracker running in `Private` or `PrivateListed` mode.
     2.1 Guest user.
         Downloaded torrents should never include any tracker key.
     2.2 Authenticated user
         2.2.1 Index can get the tracker user's key (from the Index database or generate new ones).
               - We should include the user's tracker key.
               - We should change the download torrent button label from `download torrent` to `download private torrent` (this is an Idex GUI issue).
         2.2.2 The user's tracker key is expired and the tracker API is unavailable (can't generate a new one).
               Return a `503 Service Unavailable` response with the text `Can't generate user's tracker key`.
  ```

  ### Subtasks

  - [x] Never include keys in the downloaded torrent file when the tracker is open (public or public-whitelisted).
  - [x] Never include keys in the magnet link when the tracker is open (public or public-whitelisted).
  - [x] Return an error when we can't connect to the tracker to generate the user's keys. Both for the download torrent endpoint and torrent details endpoint (magnet link).
  - [x] Return 503 instead of 500 when the tracker is offline and for example we can not generate the user's tracker keys.
  - [x] Removed duplicate URLs in the tracker lists. Both torrent file and torrent info endpoints.
  - [x] Force the main tracker URL (the one in the tracker config) to be the first in the tracker lists, both in the tracker torrent file endpoint and the torrent details endpoint.
  - [x] Panic starting the application with an invalid racker configuration. For example: private UDP tracker.

  ### Subtasks in the Index GUI

  - Rename de download torrent button from `download torrent` to `download private torrent` when the tracker is not open.

ACKs for top commit:
  josecelano:
    ACK 9b7c5c8

Tree-SHA512: d98baaf62a1643519712706a7cd4db436826a1b931e6e9172af21ac13001ac3fcad1e2d3d861b52286e8660b92b5b19bfe8206aacabe8e24024d050ffde3bd90

Cargo.toml

@@ -78,6 +78,7 @@ torrust-index-located-error = { version = "3.0.0-alpha.3-develop", path = "packa
 tower-http = { version = "0", features = ["compression-full", "cors", "trace", "propagate-header", "request-id"] }
 trace = "0.1.7"
 tracing = "0.1.40"
+url = "2.5.0"
 urlencoding = "2"
 uuid = { version = "1", features = ["v4"] }
 

src/app.rs

@@ -39,6 +39,8 @@ pub async fn run(configuration: Configuration, api_version: &Version) -> Running
 
     logging::setup(&log_level);
 
+    configuration.validate().await.expect("invalid configuration");
+
     let configuration = Arc::new(configuration);
 
     // Get configuration settings needed to build the app dependencies and

src/config.rs

@@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize};
 use thiserror::Error;
 use tokio::sync::RwLock;
 use torrust_index_located_error::{Located, LocatedError};
+use url::{ParseError, Url};
 
 /// Information required for loading config
 #[derive(Debug, Default, Clone)]
@@ -99,6 +100,17 @@ pub enum Error {
     Infallible,
 }
 
+/// Errors that can occur validating the configuration.
+#[derive(Error, Debug)]
+pub enum ValidationError {
+    /// Unable to load the configuration from the configuration file.
+    #[error("Invalid tracker URL: {source}")]
+    InvalidTrackerUrl { source: LocatedError<'static, ParseError> },
+
+    #[error("UDP private trackers are not supported. URL schemes for private tracker URLs must be HTTP ot HTTPS")]
+    UdpTrackersInPrivateModeNotSupported,
+}
+
 impl From<ConfigError> for Error {
     #[track_caller]
     fn from(err: ConfigError) -> Self {
@@ -144,6 +156,18 @@ impl Default for TrackerMode {
     }
 }
 
+impl TrackerMode {
+    #[must_use]
+    pub fn is_open(&self) -> bool {
+        matches!(self, TrackerMode::Public | TrackerMode::Whitelisted)
+    }
+
+    #[must_use]
+    pub fn is_close(&self) -> bool {
+        !self.is_open()
+    }
+}
+
 /// Configuration for the associated tracker.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct Tracker {
@@ -543,6 +567,42 @@ impl Configuration {
 
         settings_lock.net.base_url.clone()
     }
+
+    /// # Errors
+    ///
+    /// Will return an error if the configuration is invalid.
+    pub async fn validate(&self) -> Result<(), ValidationError> {
+        self.validate_tracker_config().await
+    }
+
+    /// # Errors
+    ///
+    /// Will return an error if the `tracker` configuration section is invalid.    
+    pub async fn validate_tracker_config(&self) -> Result<(), ValidationError> {
+        let settings_lock = self.settings.read().await;
+
+        let tracker_mode = settings_lock.tracker.mode.clone();
+        let tracker_url = settings_lock.tracker.url.clone();
+
+        let tracker_url = match parse_url(&tracker_url) {
+            Ok(url) => url,
+            Err(err) => {
+                return Err(ValidationError::InvalidTrackerUrl {
+                    source: Located(err).into(),
+                })
+            }
+        };
+
+        if tracker_mode.is_close() && (tracker_url.scheme() != "http" && tracker_url.scheme() != "https") {
+            return Err(ValidationError::UdpTrackersInPrivateModeNotSupported);
+        }
+
+        Ok(())
+    }
+}
+
+fn parse_url(url_str: &str) -> Result<Url, url::ParseError> {
+    Url::parse(url_str)
 }
 
 /// The public index configuration.

src/errors.rs

@@ -298,7 +298,7 @@ pub fn http_status_code_for_service_error(error: &ServiceError) -> StatusCode {
         ServiceError::InfoHashAlreadyExists => StatusCode::BAD_REQUEST,
         ServiceError::CanonicalInfoHashAlreadyExists => StatusCode::BAD_REQUEST,
         ServiceError::TorrentTitleAlreadyExists => StatusCode::BAD_REQUEST,
-        ServiceError::TrackerOffline => StatusCode::INTERNAL_SERVER_ERROR,
+        ServiceError::TrackerOffline => StatusCode::SERVICE_UNAVAILABLE,
         ServiceError::CategoryNameEmpty => StatusCode::BAD_REQUEST,
         ServiceError::CategoryAlreadyExists => StatusCode::BAD_REQUEST,
         ServiceError::TagNameEmpty => StatusCode::BAD_REQUEST,

src/models/response.rs

@@ -93,6 +93,15 @@ impl TorrentResponse {
             encoding: torrent_listing.encoding,
         }
     }
+
+    /// It adds the tracker URL in the first position of the tracker list.
+    pub fn include_url_as_main_tracker(&mut self, tracker_url: &str) {
+        // Remove any existing instances of tracker_url
+        self.trackers.retain(|tracker| tracker != tracker_url);
+
+        // Insert tracker_url at the first position
+        self.trackers.insert(0, tracker_url.to_owned());
+    }
 }
 
 #[allow(clippy::module_name_repetitions)]

src/models/torrent_file.rs

@@ -93,11 +93,45 @@ impl Torrent {
         }
     }
 
+    /// Includes the tracker URL a the main tracker in the torrent.
+    ///
+    /// It will be the URL in the `announce` field and also the first URL in the
+    /// `announce_list`.
+    pub fn include_url_as_main_tracker(&mut self, tracker_url: &str) {
+        self.set_announce_to(tracker_url);
+        self.add_url_to_front_of_announce_list(tracker_url);
+    }
+
     /// Sets the announce url to the tracker url.
     pub fn set_announce_to(&mut self, tracker_url: &str) {
         self.announce = Some(tracker_url.to_owned());
     }
 
+    /// Adds a new tracker URL to the front of the `announce_list`, removes duplicates,
+    /// and cleans up any empty inner lists.
+    ///
+    /// In practice, it's common for the `announce_list` to include the URL from
+    /// the `announce` field as one of its entries, often in the first tier,
+    /// to ensure that this primary tracker is always used. However, this is not
+    /// a strict requirement of the `BitTorrent` protocol; it's more of a
+    /// convention followed by some torrent creators for redundancy and to
+    /// ensure better availability of trackers.    
+    pub fn add_url_to_front_of_announce_list(&mut self, tracker_url: &str) {
+        if let Some(list) = &mut self.announce_list {
+            // Remove the tracker URL from existing lists
+            for inner_list in list.iter_mut() {
+                inner_list.retain(|url| url != tracker_url);
+            }
+
+            // Prepend a new vector containing the tracker_url
+            let vec = vec![tracker_url.to_owned()];
+            list.insert(0, vec);
+
+            // Remove any empty inner lists
+            list.retain(|inner_list| !inner_list.is_empty());
+        }
+    }
+
     /// Removes all other trackers if the torrent is private.
     pub fn reset_announce_list_if_private(&mut self) {
         if self.is_private() {

src/services/torrent.rs

@@ -6,7 +6,7 @@ use serde_derive::{Deserialize, Serialize};
 
 use super::category::DbCategoryRepository;
 use super::user::DbUserRepository;
-use crate::config::Configuration;
+use crate::config::{Configuration, TrackerMode};
 use crate::databases::database::{Database, Error, Sorting};
 use crate::errors::ServiceError;
 use crate::models::category::CategoryId;
@@ -257,24 +257,18 @@ impl Index {
         let mut torrent = self.torrent_repository.get_by_info_hash(info_hash).await?;
 
         let tracker_url = self.get_tracker_url().await;
+        let tracker_mode = self.get_tracker_mode().await;
 
-        // Add personal tracker url or default tracker url
-        match opt_user_id {
-            Some(user_id) => {
-                let personal_announce_url = self
-                    .tracker_service
-                    .get_personal_announce_url(user_id)
-                    .await
-                    .unwrap_or(tracker_url);
-                torrent.announce = Some(personal_announce_url.clone());
-                if let Some(list) = &mut torrent.announce_list {
-                    let vec = vec![personal_announce_url];
-                    list.insert(0, vec);
-                }
-            }
-            None => {
-                torrent.announce = Some(tracker_url);
-            }
+        // code-review: should we remove all tracker URLs in the `announce_list`
+        // when the tracker is not open?
+
+        if tracker_mode.is_open() {
+            torrent.include_url_as_main_tracker(&tracker_url);
+        } else if let Some(authenticated_user_id) = opt_user_id {
+            let personal_announce_url = self.tracker_service.get_personal_announce_url(authenticated_user_id).await?;
+            torrent.include_url_as_main_tracker(&personal_announce_url);
+        } else {
+            torrent.include_url_as_main_tracker(&tracker_url);
         }
 
         Ok(torrent)
@@ -358,24 +352,28 @@ impl Index {
 
         // Add trackers
 
+        // code-review: duplicate logic. We have to check the same in the
+        // download torrent file endpoint. Here he have only one list of tracker
+        // like the `announce_list` in the torrent file.
+
         torrent_response.trackers = self.torrent_announce_url_repository.get_by_torrent_id(&torrent_id).await?;
 
         let tracker_url = self.get_tracker_url().await;
+        let tracker_mode = self.get_tracker_mode().await;
 
-        // add tracker url
-        match opt_user_id {
-            Some(user_id) => {
-                // if no user owned tracker key can be found, use default tracker url
-                let personal_announce_url = self
-                    .tracker_service
-                    .get_personal_announce_url(user_id)
-                    .await
-                    .unwrap_or(tracker_url);
-                // add personal tracker url to front of vec
-                torrent_response.trackers.insert(0, personal_announce_url);
-            }
-            None => {
-                torrent_response.trackers.insert(0, tracker_url);
+        if tracker_mode.is_open() {
+            torrent_response.include_url_as_main_tracker(&tracker_url);
+        } else {
+            // Add main tracker URL
+            match opt_user_id {
+                Some(user_id) => {
+                    let personal_announce_url = self.tracker_service.get_personal_announce_url(user_id).await?;
+
+                    torrent_response.include_url_as_main_tracker(&personal_announce_url);
+                }
+                None => {
+                    torrent_response.include_url_as_main_tracker(&tracker_url);
+                }
             }
         }
 
@@ -513,6 +511,11 @@ impl Index {
         let settings = self.configuration.settings.read().await;
         settings.tracker.url.clone()
     }
+
+    async fn get_tracker_mode(&self) -> TrackerMode {
+        let settings = self.configuration.settings.read().await;
+        settings.tracker.mode.clone()
+    }
 }
 
 pub struct DbTorrentRepository {

tests/e2e/web/api/v1/contexts/torrent/contract.rs

@@ -201,13 +201,12 @@ mod for_guests {
             // it adds the tracker with the personal announce url, if the user
             // is logged in. If the user is not logged in, it adds the default
             // tracker again, and it ends up with two trackers.
-            trackers: vec![tracker_url.clone(), tracker_url.clone()],
+            trackers: vec![tracker_url.clone()],
             magnet_link: format!(
                 // cspell:disable-next-line
-                "magnet:?xt=urn:btih:{}&dn={}&tr={}&tr={}",
+                "magnet:?xt=urn:btih:{}&dn={}&tr={}",
                 test_torrent.file_info.info_hash.to_lowercase(),
                 urlencoding::encode(&test_torrent.index_info.title),
-                encoded_tracker_url,
                 encoded_tracker_url
             ),
             tags: vec![],
@@ -473,15 +472,6 @@ mod for_guests {
 
 mod for_authenticated_users {
 
-    use torrust_index::utils::parse_torrent::decode_torrent;
-    use torrust_index::web::api;
-
-    use crate::common::client::Client;
-    use crate::e2e::environment::TestEnv;
-    use crate::e2e::web::api::v1::contexts::torrent::asserts::{build_announce_url, get_user_tracker_key};
-    use crate::e2e::web::api::v1::contexts::torrent::steps::upload_random_torrent_to_index;
-    use crate::e2e::web::api::v1::contexts::user::steps::new_logged_in_user;
-
     mod uploading_a_torrent {
 
         use torrust_index::web::api;
@@ -779,42 +769,6 @@ mod for_authenticated_users {
         }
     }
 
-    #[tokio::test]
-    async fn it_should_allow_authenticated_users_to_download_a_torrent_with_a_personal_announce_url() {
-        let mut env = TestEnv::new();
-        env.start(api::Version::V1).await;
-
-        if !env.provides_a_tracker() {
-            println!("test skipped. It requires a tracker to be running.");
-            return;
-        }
-
-        // Given a previously uploaded torrent
-        let uploader = new_logged_in_user(&env).await;
-        let (test_torrent, _torrent_listed_in_index) = upload_random_torrent_to_index(&uploader, &env).await;
-
-        // And a logged in user who is going to download the torrent
-        let downloader = new_logged_in_user(&env).await;
-        let client = Client::authenticated(&env.server_socket_addr().unwrap(), &downloader.token);
-
-        // When the user downloads the torrent
-        let response = client.download_torrent(&test_torrent.file_info_hash()).await;
-
-        let torrent = decode_torrent(&response.bytes).expect("could not decode downloaded torrent");
-
-        // Then the torrent should have the personal announce URL
-        let tracker_key = get_user_tracker_key(&downloader, &env)
-            .await
-            .expect("uploader should have a valid tracker key");
-
-        let tracker_url = env.server_settings().unwrap().tracker.url;
-
-        assert_eq!(
-            torrent.announce.unwrap(),
-            build_announce_url(&tracker_url, &Some(tracker_key))
-        );
-    }
-
     mod and_non_admins {
 
         use torrust_index::web::api;