Repository: lldap/lldap (language: rust)

Issue:
[BUG] Treat `database_url` as secret
**Describe the bug**

When enabling the verbose-mode (`LLDAP_VERBOSE=true`), you get the configuration pasted to the log, but this contains the `database_url` in plaintext, although it might contain the database credentials :thinking: The verbose-mode should not be enabled in production, but maybe it might make sense to handle the `database_url` as secret too, like for `jwt_secret` and `ldap_user_pass`.

**To Reproduce**

Start LLDAP all required ENV-variables and these:

```sh
export LLDAP_VERBOSE=true
export LLDAP_DATABASE_URL=postgres://user:password@host:port/database
```

**Expected behavior**

I would've expected the `database_url` to be handled as secret :thinking: 

```rust
Configuration: Configuration {
  // ...
  database_url: ***SECRET***,
  // ...
}
```

**Logs**

Currently I get this:

```rust
Configuration: Configuration {
  // ...
  jwt_secret: ***SECRET***,
  // ...
  ldap_user_pass: ***SECRET***,
  // ...
  database_url: "postgres://user:password@host:port/database",
  // ...
}
```

**Additional context**

LLDAP-Version: `lldap/lldap:2023-11-05-alpine`


Discussion / hints:
Hmm, I'm a bit conflicted on that one: I agree that the password should be secret. However, it's usually important to know which database you're talking to.

I wonder if I can just parse it as a URL, would it be equivalent? Then I can consider the various parts and mask the password.
Masking only the password would also be a possible way to go :+1: 

Fix patch:
diff --git a/server/src/infra/cli.rs b/server/src/infra/cli.rs
index 1aa8ad2b..c0f5ca78 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 2400ebcc..c6c81292 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 00000000..6517272e
--- /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 d81e83ab..38301ad3 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 560a77d9..fc095f2c 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)


Test patch: