Skip to content

Commit 1a38038

Browse files
glambersonlamco-office
authored andcommitted
fix(server): address Copilot review on CredentialValidator
- Skip validation when credentials are None (reactivation, CredSSP) instead of bailing, consistent with #1150/#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 6a665ad commit 1a38038

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
@@ -49,7 +49,8 @@ const LISTENER_BACKLOG: u32 = 1024;
4949
/// pre-loaded credentials for NTLM challenge-response).
5050
///
5151
/// Implement this trait to validate credentials against external systems
52-
/// (PAM, LDAP, database, etc.).
52+
/// (PAM, LDAP, database, etc.). For blocking backends, wrap the call in
53+
/// `tokio::task::spawn_blocking` to avoid stalling the async runtime.
5354
pub trait CredentialValidator: Send + Sync {
5455
/// Validate credentials received from the client.
5556
/// Return `Ok(true)` to accept, `Ok(false)` to reject.
@@ -905,20 +906,19 @@ impl RdpServer {
905906
if let Some(creds) = &result.credentials {
906907
match validator.validate(creds) {
907908
Ok(true) => {
908-
debug!("Credential validation succeeded for user: {}", creds.username);
909+
debug!("Credential validation succeeded");
909910
}
910911
Ok(false) => {
911-
warn!("Credential validation failed for user: {}", creds.username);
912+
warn!("Credential validation failed");
912913
bail!("credential validation failed");
913914
}
914915
Err(e) => {
915-
error!("Credential validator error: {e:#}");
916-
bail!("credential validation error: {e}");
916+
error!(error = %e, "Credential validator error");
917+
bail!("credential validation error");
917918
}
918919
}
919920
} else {
920-
warn!("Credential validator configured but no credentials received from client");
921-
bail!("no credentials received for validation");
921+
debug!("Skipping credential validation (no credentials in AcceptorResult)");
922922
}
923923
}
924924

0 commit comments

Comments
 (0)