Skip to content

Commit fe37ac3

Browse files
committed
fix(server): address Copilot review on CredentialValidator
- Skip validation when credentials are None (reactivation, CredSSP) instead of bailing, consistent with Devolutions#1150/Devolutions#1155 design - Remove username from log messages to avoid leaking sensitive data - Keep validator error details in structured tracing field only - Add spawn_blocking guidance to trait doc for blocking backends
1 parent 2743a83 commit fe37ac3

1 file changed

Lines changed: 7 additions & 7 deletions

File tree

crates/ironrdp-server/src/server.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ pub trait ConnectionHandler: Send {
9494
/// pre-loaded credentials for NTLM challenge-response).
9595
///
9696
/// Implement this trait to validate credentials against external systems
97-
/// (PAM, LDAP, database, etc.).
97+
/// (PAM, LDAP, database, etc.). For blocking backends, wrap the call in
98+
/// `tokio::task::spawn_blocking` to avoid stalling the async runtime.
9899
pub trait CredentialValidator: Send + Sync {
99100
/// Validate credentials received from the client.
100101
/// Return `Ok(true)` to accept, `Ok(false)` to reject.
@@ -988,20 +989,19 @@ impl RdpServer {
988989
if let Some(creds) = &result.credentials {
989990
match validator.validate(creds) {
990991
Ok(true) => {
991-
debug!("Credential validation succeeded for user: {}", creds.username);
992+
debug!("Credential validation succeeded");
992993
}
993994
Ok(false) => {
994-
warn!("Credential validation failed for user: {}", creds.username);
995+
warn!("Credential validation failed");
995996
bail!("credential validation failed");
996997
}
997998
Err(e) => {
998-
error!("Credential validator error: {e:#}");
999-
bail!("credential validation error: {e}");
999+
error!(error = %e, "Credential validator error");
1000+
bail!("credential validation error");
10001001
}
10011002
}
10021003
} else {
1003-
warn!("Credential validator configured but no credentials received from client");
1004-
bail!("no credentials received for validation");
1004+
debug!("Skipping credential validation (no credentials in AcceptorResult)");
10051005
}
10061006
}
10071007

0 commit comments

Comments
 (0)