Skip to content

Commit

Permalink
Merge #479: Remove secrets from settings API endpoint
Browse files Browse the repository at this point in the history
e341e98 feat: [#424] remove secrets from settings API endpoint (Jose Celano)

Pull request description:

  These fields:

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

  are replaced with asterisks. Final output:

  ```json
  {
    "log_level": "info",
    "website": {
      "name": "Torrust"
    },
    "tracker": {
      "url": "udp://localhost:6969",
      "mode": "Public",
      "api_url": "http://localhost:1212",
      "token": "***",
      "token_valid_seconds": 7257600
    },
    "net": {
      "port": 3001,
      "base_url": null
    },
    "auth": {
      "email_on_signup": "Optional",
      "min_password_length": 6,
      "max_password_length": 64,
      "secret_key": "***"
    },
    "database": {
      "connect_url": "***"
    },
    "mail": {
      "email_verification_enabled": false,
      "from": "[email protected]",
      "reply_to": "[email protected]",
      "username": "",
      "password": "***",
      "server": "",
      "port": 25
    },
    "image_cache": {
      "max_request_timeout_ms": 1000,
      "capacity": 128000000,
      "entry_size_limit": 4000000,
      "user_quota_period_seconds": 3600,
      "user_quota_bytes": 64000000
    },
    "api": {
      "default_torrent_page_size": 10,
      "max_torrent_page_size": 30
    },
    "tracker_statistics_importer": {
      "torrent_info_update_interval": 3600,
      "port": 3002
    }
  }
  ```

ACKs for top commit:
  josecelano:
    ACK e341e98

Tree-SHA512: 696091205ece046f5242b58222c538cb567210edac1db6f58e7cc1a81fa17f21c78857b8d691ac56f2f152f69e4add2b76fe78bafddc1a07a1d0a107af1e161c
  • Loading branch information
josecelano committed Feb 9, 2024
2 parents 1388e76 + e341e98 commit 286d9cf
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 3 deletions.
7 changes: 7 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 24 additions & 1 deletion src/services/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/web/api/server/v1/contexts/settings/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
};
Expand Down
15 changes: 15 additions & 0 deletions tests/e2e/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/web/api/v1/contexts/settings/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit 286d9cf

Please sign in to comment.