feat: [#424] remove secrets from settings API endpoint

josecelano · josecelano · commit e341e98bf09c · 2024-02-09T11:56:55.000Z
These fields:

- data.tracker.token
- data.database.connect_url
- data.mail.password
- data.auth.secret_key

are replaced with asterisks.
diff --git a/src/config.rs b/src/config.rs
@@ -389,6 +389,13 @@ impl TorrustIndex {
     fn override_tracker_api_token(&mut self, tracker_api_token: &str) {
         self.tracker.override_tracker_api_token(tracker_api_token);
     }
+
+    pub fn remove_secrets(&mut self) {
+        self.tracker.token = "***".to_owned();
+        self.database.connect_url = "***".to_owned();
+        self.mail.password = "***".to_owned();
+        self.auth.secret_key = "***".to_owned();
+    }
 }
 
 /// The configuration service.
diff --git a/src/services/settings.rs b/src/services/settings.rs
@@ -34,7 +34,30 @@ impl Service {
             return Err(ServiceError::Unauthorized);
         }
 
-        Ok(self.configuration.get_all().await)
+        let torrust_index_configuration = self.configuration.get_all().await;
+
+        Ok(torrust_index_configuration)
+    }
+
+    /// It gets all the settings making the secrets with asterisks.
+    ///
+    /// # Errors
+    ///
+    /// It returns an error if the user does not have the required permissions.
+    pub async fn get_all_masking_secrets(&self, user_id: &UserId) -> Result<TorrustIndex, ServiceError> {
+        let user = self.user_repository.get_compact(user_id).await?;
+
+        // Check if user is administrator
+        // todo: extract authorization service
+        if !user.administrator {
+            return Err(ServiceError::Unauthorized);
+        }
+
+        let mut torrust_index_configuration = self.configuration.get_all().await;
+
+        torrust_index_configuration.remove_secrets();
+
+        Ok(torrust_index_configuration)
     }
 
     /// It gets only the public settings.
diff --git a/src/web/api/server/v1/contexts/settings/handlers.rs b/src/web/api/server/v1/contexts/settings/handlers.rs
@@ -22,7 +22,7 @@ pub async fn get_all_handler(State(app_data): State<Arc<AppData>>, Extract(maybe
         Err(error) => return error.into_response(),
     };
 
-    let all_settings = match app_data.settings_service.get_all(&user_id).await {
+    let all_settings = match app_data.settings_service.get_all_masking_secrets(&user_id).await {
         Ok(all_settings) => all_settings,
         Err(error) => return error.into_response(),
     };
diff --git a/tests/e2e/environment.rs b/tests/e2e/environment.rs
@@ -85,6 +85,21 @@ impl TestEnv {
         self.starting_settings.clone()
     }
 
+    /// Returns the server starting settings if the servers was already started,
+    /// masking secrets with asterisks.
+    pub fn server_settings_masking_secrets(&self) -> Option<Settings> {
+        match self.starting_settings.clone() {
+            Some(mut settings) => {
+                settings.tracker.token = "***".to_owned();
+                settings.database.connect_url = "***".to_owned();
+                settings.mail.password = "***".to_owned();
+                settings.auth.secret_key = "***".to_owned();
+                Some(settings)
+            }
+            None => None,
+        }
+    }
+
     /// Provides the API server socket address.
     /// For example: `localhost:3001`.
     pub fn server_socket_addr(&self) -> Option<String> {
diff --git a/tests/e2e/web/api/v1/contexts/settings/contract.rs b/tests/e2e/web/api/v1/contexts/settings/contract.rs
@@ -61,7 +61,7 @@ async fn it_should_allow_admins_to_get_all_the_settings() {
 
     let res: AllSettingsResponse = serde_json::from_str(&response.body).unwrap();
 
-    assert_eq!(res.data, env.server_settings().unwrap());
+    assert_eq!(res.data, env.server_settings_masking_secrets().unwrap());
 
     assert_json_ok_response(&response);
 }

Original file line number	Diff line number	Diff line change
`@@ -389,6 +389,13 @@ impl TorrustIndex {`
`389`	`389`	`fn override_tracker_api_token(&mut self, tracker_api_token: &str) {`
`390`	`390`	`self.tracker.override_tracker_api_token(tracker_api_token);`
`391`	`391`	`}`
	`392`	`+`
	`393`	`+ pub fn remove_secrets(&mut self) {`
	`394`	`+ self.tracker.token = "***".to_owned();`
	`395`	`+ self.database.connect_url = "***".to_owned();`
	`396`	`+ self.mail.password = "***".to_owned();`
	`397`	`+ self.auth.secret_key = "***".to_owned();`
	`398`	`+ }`
`392`	`399`	`}`
`393`	`400`
`394`	`401`	`/// The configuration service.`
Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ async fn it_should_allow_admins_to_get_all_the_settings() {`
`61`	`61`
`62`	`62`	`let res: AllSettingsResponse = serde_json::from_str(&response.body).unwrap();`
`63`	`63`
`64`		`- assert_eq!(res.data, env.server_settings().unwrap());`
	`64`	`+ assert_eq!(res.data, env.server_settings_masking_secrets().unwrap());`
`65`	`65`
`66`	`66`	`assert_json_ok_response(&response);`
`67`	`67`	`}`