From b82a2d570584e2dba964f005e5fbee3f4f496c3c Mon Sep 17 00:00:00 2001
From: Valentin Tolmer <valentin@tolmer.fr>
Date: Mon, 22 Jan 2024 23:02:30 +0100
Subject: [PATCH] server: Treat the database password as a secret

---
 server/src/infra/cli.rs             |  4 ++-
 server/src/infra/configuration.rs   | 11 +++---
 server/src/infra/database_string.rs | 54 +++++++++++++++++++++++++++++
 server/src/infra/mod.rs             |  1 +
 server/src/main.rs                  |  7 ++--
 5 files changed, 69 insertions(+), 8 deletions(-)
 create mode 100644 server/src/infra/database_string.rs

diff --git a/server/src/infra/cli.rs b/server/src/infra/cli.rs
index 1aa8ad2..c0f5ca7 100644
--- a/server/src/infra/cli.rs
+++ b/server/src/infra/cli.rs
@@ -3,6 +3,8 @@ use lettre::message::Mailbox;
 use serde::{Deserialize, Serialize};
 use url::Url;
 
+use crate::infra::database_string::DatabaseUrl;
+
 /// lldap is a lightweight LDAP server
 #[derive(Debug, Parser, Clone)]
 #[clap(version, author)]
@@ -87,7 +89,7 @@ pub struct RunOpts {
 
     /// Database connection URL
     #[clap(short, long, env = "LLDAP_DATABASE_URL")]
-    pub database_url: Option<String>,
+    pub database_url: Option<DatabaseUrl>,
 
     /// Force admin password reset to the config value.
     #[clap(long, env = "LLDAP_FORCE_LADP_USER_PASS_RESET")]
diff --git a/server/src/infra/configuration.rs b/server/src/infra/configuration.rs
index 2400ebc..c6c8129 100644
--- a/server/src/infra/configuration.rs
+++ b/server/src/infra/configuration.rs
@@ -3,7 +3,10 @@ use crate::{
         sql_tables::{ConfigLocation, PrivateKeyHash, PrivateKeyInfo, PrivateKeyLocation},
         types::{AttributeName, UserId},
     },
-    infra::cli::{GeneralConfigOpts, LdapsOpts, RunOpts, SmtpEncryption, SmtpOpts, TestEmailOpts},
+    infra::{
+        cli::{GeneralConfigOpts, LdapsOpts, RunOpts, SmtpEncryption, SmtpOpts, TestEmailOpts},
+        database_string::DatabaseUrl,
+    },
 };
 use anyhow::{bail, Context, Result};
 use figment::{
@@ -91,8 +94,8 @@ pub struct Configuration {
     pub force_ldap_user_pass_reset: bool,
     #[builder(default = "false")]
     pub force_update_private_key: bool,
-    #[builder(default = r#"String::from("sqlite://users.db?mode=rwc")"#)]
-    pub database_url: String,
+    #[builder(default = r#"DatabaseUrl::from("sqlite://users.db?mode=rwc")"#)]
+    pub database_url: DatabaseUrl,
     #[builder(default)]
     pub ignored_user_attributes: Vec<AttributeName>,
     #[builder(default)]
@@ -411,7 +414,7 @@ impl ConfigOverrider for RunOpts {
         }
 
         if let Some(database_url) = self.database_url.as_ref() {
-            config.database_url = database_url.to_string();
+            config.database_url = database_url.clone();
         }
 
         if let Some(force_ldap_user_pass_reset) = self.force_ldap_user_pass_reset {
diff --git a/server/src/infra/database_string.rs b/server/src/infra/database_string.rs
new file mode 100644
index 0000000..6517272
--- /dev/null
+++ b/server/src/infra/database_string.rs
@@ -0,0 +1,54 @@
+use serde::{Deserialize, Serialize};
+use url::Url;
+
+#[derive(Clone, Serialize, Deserialize)]
+pub struct DatabaseUrl(Url);
+
+impl From<Url> for DatabaseUrl {
+    fn from(url: Url) -> Self {
+        Self(url)
+    }
+}
+
+impl From<&str> for DatabaseUrl {
+    fn from(url: &str) -> Self {
+        Self(Url::parse(url).expect("Invalid database URL"))
+    }
+}
+
+impl std::fmt::Debug for DatabaseUrl {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        if self.0.password().is_some() {
+            let mut url = self.0.clone();
+            // It can fail for URLs that cannot have a password, like "mailto:bob@example".
+            let _ = url.set_password(Some("***PASSWORD***"));
+            f.write_fmt(format_args!("{}", url))
+        } else {
+            f.write_fmt(format_args!("{}", self.0))
+        }
+    }
+}
+
+impl ToString for DatabaseUrl {
+    fn to_string(&self) -> String {
+        self.0.to_string()
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_database_url_debug() {
+        let url = DatabaseUrl::from("postgres://user:pass@localhost:5432/dbname");
+        assert_eq!(
+            format!("{:?}", url),
+            "postgres://user:***PASSWORD***@localhost:5432/dbname"
+        );
+        assert_eq!(
+            url.to_string(),
+            "postgres://user:pass@localhost:5432/dbname"
+        );
+    }
+}
diff --git a/server/src/infra/mod.rs b/server/src/infra/mod.rs
index d81e83a..38301ad 100644
--- a/server/src/infra/mod.rs
+++ b/server/src/infra/mod.rs
@@ -2,6 +2,7 @@ pub mod access_control;
 pub mod auth_service;
 pub mod cli;
 pub mod configuration;
+pub mod database_string;
 pub mod db_cleaner;
 pub mod graphql;
 pub mod healthcheck;
diff --git a/server/src/main.rs b/server/src/main.rs
index 560a77d..fc095f2 100644
--- a/server/src/main.rs
+++ b/server/src/main.rs
@@ -18,6 +18,7 @@ use crate::{
     infra::{
         cli::*,
         configuration::{compare_private_key_hashes, Configuration},
+        database_string::DatabaseUrl,
         db_cleaner::Scheduler,
         healthcheck, mail,
     },
@@ -84,7 +85,7 @@ async fn set_up_server(config: Configuration) -> Result<ServerBuilder> {
     info!("Starting LLDAP version {}", env!("CARGO_PKG_VERSION"));
 
     let sql_pool = {
-        let mut sql_opt = sea_orm::ConnectOptions::new(config.database_url.clone());
+        let mut sql_opt = sea_orm::ConnectOptions::new(config.database_url.to_string());
         sql_opt
             .max_connections(5)
             .sqlx_logging(true)
@@ -248,9 +249,9 @@ fn run_healthcheck(opts: RunOpts) -> Result<()> {
     std::process::exit(i32::from(failure))
 }
 
-async fn create_schema(database_url: String) -> Result<()> {
+async fn create_schema(database_url: DatabaseUrl) -> Result<()> {
     let sql_pool = {
-        let mut sql_opt = sea_orm::ConnectOptions::new(database_url.clone());
+        let mut sql_opt = sea_orm::ConnectOptions::new(database_url.to_string());
         sql_opt
             .max_connections(1)
             .sqlx_logging(true)