feat: log which auth error the user experienced to stderr (#1009)

Author: levkk

pgdog/src/auth/auth_result.rs

@@ -0,0 +1,51 @@
+use std::fmt::Display;
+
+#[derive(Default, PartialEq, Debug, Clone, Copy)]
+pub enum AuthResult {
+    /// No problems.
+    #[default]
+    Ok,
+    /// Password provided by user doesn't match config.
+    NoPasswordMatch,
+    /// Passwords not configured.
+    NoPasswordConfig,
+    /// User identity (TLS cert) doesn't match configured identity.
+    NoIdentity,
+    /// Passthrough auth says user doesn't exist.
+    NoPassthroughNoUser,
+    /// Passthrough auth doesn't allow password changes.
+    NoPassthroughPasswordChange,
+    /// No user or database in config.
+    NoUserOrDatabase,
+    /// Client didn't provide password message.
+    NoPasswordMessage,
+}
+
+impl AuthResult {
+    pub fn is_ok(&self) -> bool {
+        matches!(self, Self::Ok)
+    }
+}
+
+impl PartialEq<bool> for AuthResult {
+    fn eq(&self, other: &bool) -> bool {
+        self.is_ok() == *other
+    }
+}
+
+impl Display for AuthResult {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Ok => write!(f, "auth ok"),
+            Self::NoPasswordMatch => write!(f, "wrong password"),
+            Self::NoPasswordConfig => write!(f, "user has no passwords in config"),
+            Self::NoIdentity => write!(f, "user identity does not match certificate"),
+            Self::NoPassthroughNoUser => write!(f, "no user in config (passthrough auth)"),
+            Self::NoPassthroughPasswordChange => {
+                write!(f, "passthrough auth does not allow password change")
+            }
+            Self::NoUserOrDatabase => write!(f, "no user or database in config"),
+            Self::NoPasswordMessage => write!(f, "client did not send password message"),
+        }
+    }
+}

pgdog/src/auth/mod.rs

@@ -1,8 +1,10 @@
 //! PostgreSQL authentication mechanisms.
 
+pub mod auth_result;
 pub mod error;
 pub mod md5;
 pub mod scram;
 
+pub use auth_result::AuthResult;
 pub use error::Error;
 pub use md5::Client;

pgdog/src/backend/databases.rs

@@ -12,6 +12,7 @@ use parking_lot::{Mutex, RawMutex};
 use pgdog_config::users::PasswordKind;
 use tracing::{debug, error, info, warn};
 
+use crate::auth::AuthResult;
 use crate::backend::replication::ShardedSchemas;
 use crate::config::PoolerMode;
 use crate::frontend::client::query_engine::two_pc::Manager;
@@ -154,7 +155,7 @@ pub fn reload() -> Result<(), Error> {
 ///
 /// Return true if user can login, false otherwise.
 ///
-pub(crate) fn add(user: ConfigUser) -> Result<bool, Error> {
+pub(crate) fn add(user: ConfigUser) -> Result<AuthResult, Error> {
     fn add_user(user: ConfigUser) -> Result<(), Error> {
         debug!(
             r#"adding user "{}" to database "{}" via passthrough auth"#,
@@ -179,23 +180,23 @@ pub(crate) fn add(user: ConfigUser) -> Result<bool, Error> {
             existing.password = user.password.clone();
             add_user(existing)?;
             reload_from_existing()?;
-            Ok(true)
+            Ok(AuthResult::Ok)
         } else if existing.password == user.password {
             // Passwords match.
-            Ok(true)
+            Ok(AuthResult::Ok)
         } else if config.config.general.passthrough_auth.allows_change() {
             // Passwords don't match but we can change it.
             existing.password = user.password.clone();
             add_user(user)?;
             reload_from_existing()?;
-            Ok(true)
+            Ok(AuthResult::Ok)
         } else {
-            Ok(false)
+            Ok(AuthResult::NoPassthroughPasswordChange)
         }
     } else {
         add_user(user)?;
         reload_from_existing()?;
-        Ok(true)
+        Ok(AuthResult::Ok)
     }
 }
 
@@ -773,7 +774,7 @@ mod tests {
 
         let result = add(make_user("new_user", Some("secret")));
         assert!(result.is_ok());
-        assert!(result.unwrap());
+        assert!(result.unwrap().is_ok());
 
         let config = crate::config::config();
         let found = config.users.find(&make_user("new_user", None));
@@ -790,7 +791,7 @@ mod tests {
 
         let result = add(make_user("alice", Some("pass123")));
         assert!(result.is_ok());
-        assert!(result.unwrap());
+        assert!(result.unwrap().is_ok());
     }
 
     #[tokio::test]
@@ -802,7 +803,7 @@ mod tests {
 
         let result = add(make_user("bob", Some("new_pass")));
         assert!(result.is_ok());
-        assert!(result.unwrap());
+        assert!(result.unwrap().is_ok());
 
         let config = crate::config::config();
         let found = config.users.find(&make_user("bob", None));
@@ -818,7 +819,7 @@ mod tests {
 
         let result = add(make_user("charlie", Some("wrong_pass")));
         assert!(result.is_ok());
-        assert!(!result.unwrap());
+        assert!(!result.unwrap().is_ok());
     }
 
     #[tokio::test]
@@ -830,7 +831,7 @@ mod tests {
 
         let result = add(make_user("dave", Some("new_pass")));
         assert!(result.is_ok());
-        assert!(result.unwrap());
+        assert!(result.unwrap().is_ok());
 
         let config = crate::config::config();
         let found = config.users.find(&make_user("dave", None));

pgdog/src/frontend/client/mod.rs

@@ -7,9 +7,10 @@ use std::time::{Duration, Instant};
 use pgdog_config::users::PasswordKind;
 use timeouts::Timeouts;
 use tokio::{select, spawn, time::timeout};
-use tracing::{debug, enabled, error, info, trace, Level as LogLevel};
+use tracing::{debug, enabled, error, info, trace, warn, Level as LogLevel};
 
 use super::{ClientRequest, Error, PreparedStatements};
+use crate::auth::AuthResult;
 use crate::auth::{md5, scram::Server};
 use crate::backend::maintenance_mode;
 use crate::backend::pool::stats::MemoryStats;
@@ -144,12 +145,12 @@ impl Client {
         user: &str,
         auth_type: &AuthType,
         passwords: &[PasswordKind],
-    ) -> Result<bool, Error> {
+    ) -> Result<AuthResult, Error> {
         if passwords.is_empty() {
-            return Ok(false);
+            return Ok(AuthResult::NoPasswordConfig);
         }
 
-        let ok = match auth_type {
+        let result = match auth_type {
             AuthType::Md5 => {
                 let md5 = md5::Client::new(
                     user,
@@ -158,9 +159,13 @@ impl Client {
                 stream.send_flush(&md5.challenge()).await?;
                 let password = Password::from_bytes(stream.read().await?.to_bytes()?)?;
                 if let Password::PasswordMessage { response } = password {
-                    md5.check(&response)
+                    if md5.check(&response) {
+                        AuthResult::Ok
+                    } else {
+                        AuthResult::NoPasswordMatch
+                    }
                 } else {
-                    false
+                    AuthResult::NoPasswordMessage
                 }
             }
 
@@ -169,7 +174,11 @@ impl Client {
 
                 let scram = Server::new(passwords);
                 let res = scram.handle(stream).await;
-                matches!(res, Ok(true))
+                if matches!(res, Ok(true)) {
+                    AuthResult::Ok
+                } else {
+                    AuthResult::NoPasswordMatch
+                }
             }
 
             AuthType::Plain => {
@@ -178,15 +187,21 @@ impl Client {
                     .await?;
                 let response = stream.read().await?;
                 let response = Password::from_bytes(response.to_bytes()?)?;
-                passwords
+                let is_match = passwords
                     .iter()
-                    .any(|p| Some(p.as_str()) == response.password())
+                    .any(|p| Some(p.as_str()) == response.password());
+
+                if is_match {
+                    AuthResult::Ok
+                } else {
+                    AuthResult::NoPasswordMatch
+                }
             }
 
-            AuthType::Trust => true,
+            AuthType::Trust => AuthResult::Ok,
         };
 
-        Ok(ok)
+        Ok(result)
     }
 
     /// Create new frontend client from the given TCP stream.
@@ -210,13 +225,14 @@ impl Client {
         let passthrough = config.config.general.passthrough_auth();
         let id = BackendKeyData::new_client(protocol_version);
         let comms = ClientComms::new(&id);
+        let log_connections = config.config.general.log_connections;
 
         // Check if we need to ask the client for its password in plaintext
         // because we don't actually have it configured.
         //
         // This is likely because passthrough authentication is enabled.
         //
-        let auth_ok = if passthrough {
+        let auth_result = if passthrough {
             // Get the password. We always need it because we need to check if
             // it's current and hasn't been changed.
             stream
@@ -232,7 +248,7 @@ impl Client {
             if let Some(user) = user {
                 databases::add(user)?
             } else {
-                false
+                AuthResult::NoPassthroughNoUser
             }
         } else if admin {
             // The admin database is virtual and never present in the cluster
@@ -245,19 +261,29 @@ impl Client {
                     if let Some(identity) = cluster.identity() {
                         // mTLS authentication: the client certificate identity
                         // must match the configured user identity.
-                        stream.tls_identity() == Some(identity)
+                        if stream.tls_identity() == Some(identity) {
+                            AuthResult::Ok
+                        } else {
+                            AuthResult::NoIdentity
+                        }
                     } else {
                         // Password authentication.
                         Self::check_password(&mut stream, user, auth_type, cluster.passwords())
                             .await?
                     }
                 }
 
-                Err(_) => false,
+                Err(_) => AuthResult::NoUserOrDatabase,
             }
         };
 
-        if !auth_ok {
+        if !auth_result.is_ok() {
+            if log_connections {
+                warn!(
+                    r#"user "{}" and database "{}" auth error: {}"#,
+                    user, database, auth_result
+                );
+            }
             stream.fatal(ErrorResponse::auth(user, database)).await?;
             return Ok(None);
         } else {