RUST-847 Redact Credential Debug output (1.2.x) (#357)

Author: patrickfreed

src/client/auth/mod.rs

@@ -10,7 +10,7 @@ mod scram;
 mod test;
 mod x509;
 
-use std::{borrow::Cow, str::FromStr};
+use std::{borrow::Cow, fmt::Debug, str::FromStr};
 
 use hmac::Mac;
 use rand::Rng;
@@ -324,7 +324,7 @@ impl FromStr for AuthMechanism {
 ///
 /// Some fields (mechanism and source) may be omitted and will either be negotiated or assigned a
 /// default value, depending on the values of other fields in the credential.
-#[derive(Clone, Debug, Default, Deserialize, TypedBuilder, PartialEq)]
+#[derive(Clone, Default, Deserialize, TypedBuilder, PartialEq)]
 #[non_exhaustive]
 pub struct Credential {
     /// The username to authenticate with. This applies to all mechanisms but may be omitted when
@@ -437,6 +437,14 @@ impl Credential {
     }
 }
 
+impl Debug for Credential {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_tuple("Credential")
+            .field(&"REDACTED".to_string())
+            .finish()
+    }
+}
+
 /// Contains the first client message sent as part of the authentication handshake.
 pub(crate) enum ClientFirst {
     Scram(ScramVersion, scram::ClientFirst),

src/client/options/mod.rs

@@ -1080,7 +1080,7 @@ impl ClientOptionsParser {
                     None
                 } else {
                     let port_string_without_colon = &port[1..];
-                    let p = u16::from_str_radix(port_string_without_colon, 10).map_err(|_| {
+                    let p: u16 = port_string_without_colon.parse().map_err(|_| {
                         ErrorKind::ArgumentError {
                             message: format!(
                                 "invalid port specified in connection string: {}",
@@ -1325,7 +1325,7 @@ impl ClientOptionsParser {
 
         macro_rules! get_duration {
             ($value:expr, $option:expr) => {
-                match u64::from_str_radix($value, 10) {
+                match $value.parse::<u64>() {
                     Ok(i) => i,
                     _ => {
                         return Err(ErrorKind::ArgumentError {
@@ -1342,7 +1342,7 @@ impl ClientOptionsParser {
 
         macro_rules! get_u32 {
             ($value:expr, $option:expr) => {
-                match u32::from_str_radix(value, 10) {
+                match value.parse::<u32>() {
                     Ok(u) => u,
                     Err(_) => {
                         return Err(ErrorKind::ArgumentError {
@@ -1359,7 +1359,7 @@ impl ClientOptionsParser {
 
         macro_rules! get_i32 {
             ($value:expr, $option:expr) => {
-                match i32::from_str_radix(value, 10) {
+                match value.parse::<i32>() {
                     Ok(u) => u,
                     Err(_) => {
                         return Err(ErrorKind::ArgumentError {
@@ -1631,7 +1631,7 @@ impl ClientOptionsParser {
             "w" => {
                 let mut write_concern = self.write_concern.get_or_insert_with(Default::default);
 
-                match i32::from_str_radix(value, 10) {
+                match value.parse::<i32>() {
                     Ok(w) => {
                         if w < 0 {
                             return Err(ErrorKind::ArgumentError {

src/cmap/establish/handshake/mod.rs

@@ -224,8 +224,8 @@ impl Handshaker {
         });
 
         Ok(HandshakeResult {
-            first_round,
             is_master_reply,
+            first_round,
         })
     }
 }

src/cmap/mod.rs

@@ -86,9 +86,9 @@ impl ConnectionPool {
             address,
             manager,
             connection_requester,
+            generation_subscriber,
             wait_queue_timeout,
             event_handler,
-            generation_subscriber,
         }
     }
 

src/cmap/test/mod.rs

@@ -20,12 +20,15 @@ use crate::{
     test::{
         assert_matches,
         run_spec_test,
+        CmapEvent,
         EventClient,
         Matchable,
+        TestClient,
         CLIENT_OPTIONS,
         LOCK,
         SERVER_API,
     },
+    Client,
     RUNTIME,
 };
 use bson::doc;
@@ -435,3 +438,46 @@ async fn cmap_spec_tests() {
 
     run_spec_test(&["connection-monitoring-and-pooling"], run_cmap_spec_tests).await;
 }
+
+#[cfg_attr(feature = "tokio-runtime", tokio::test)]
+#[cfg_attr(feature = "async-std-runtime", async_std::test)]
+async fn redact_credential() {
+    let _lock = LOCK.run_concurrently().await;
+
+    let client = TestClient::new().await;
+    if !client.auth_enabled() {
+        println!("skipping redact_credential due to auth not being enabled");
+        return;
+    }
+
+    let mut options = CLIENT_OPTIONS.clone();
+    options.direct_connection = Some(true);
+    options.hosts.drain(1..);
+
+    let credential = options.credential.clone().unwrap();
+
+    let handler = Arc::new(crate::test::EventHandler::new());
+    let mut subscriber = handler.subscribe();
+    options.cmap_event_handler = Some(handler.clone());
+
+    let _client = Client::with_options(options).unwrap();
+
+    subscriber
+        .wait_for_event(EVENT_TIMEOUT, |e| {
+            if let crate::test::Event::CmapEvent(CmapEvent::ConnectionPoolCreated(event)) = e {
+                let event_debug = format!("{:?}", event);
+                if let Some(ref pass) = credential.password {
+                    assert!(!event_debug.contains(pass))
+                }
+                if let Some(ref user) = credential.username {
+                    assert!(!event_debug.contains(user))
+                }
+                assert!(event_debug.contains("REDACTED"));
+                true
+            } else {
+                false
+            }
+        })
+        .await
+        .expect("expected connection pool created event");
+}

src/coll/options.rs

@@ -449,8 +449,8 @@ pub struct FindOneAndUpdateOptions {
 /// Specifies the options to a [`Collection::aggregate`](../struct.Collection.html#method.aggregate)
 /// operation.
 #[skip_serializing_none]
-#[serde(rename_all = "camelCase")]
 #[derive(Clone, Debug, Default, Deserialize, TypedBuilder, Serialize)]
+#[serde(rename_all = "camelCase")]
 #[non_exhaustive]
 pub struct AggregateOptions {
     /// Enables writing to temporary files. When set to true, aggregation stages can write data to

src/test/spec/unified_runner/mod.rs

@@ -13,7 +13,7 @@ use tokio::sync::RwLockWriteGuard;
 use crate::{
     bson::{doc, Document},
     options::{CollectionOptions, FindOptions, ReadConcern, ReadPreference, SelectionCriteria},
-    test::{run_spec_test, LOCK},
+    test::{run_spec_test, TestClient, LOCK},
 };
 
 pub use self::{
@@ -245,5 +245,10 @@ async fn test_examples() {
 #[cfg_attr(feature = "async-std-runtime", async_std::test)]
 async fn test_versioned_api() {
     let _guard: RwLockWriteGuard<_> = LOCK.run_exclusively().await;
+
+    // TODO RUST-725 Unskip these tests on 5.0
+    if TestClient::new().await.server_version_gte(5, 0) {
+        return;
+    }
     run_spec_test(&["versioned-api"], run_unified_format_test).await;
 }

src/test/util/event.rs

@@ -14,7 +14,7 @@ use crate::{
     bson::doc,
     client::options::ServerApi,
     event::{
-        cmap::{CmapEventHandler, PoolClearedEvent, PoolReadyEvent},
+        cmap::{CmapEventHandler, PoolClearedEvent, PoolCreatedEvent, PoolReadyEvent},
         command::{
             CommandEventHandler,
             CommandFailedEvent,
@@ -107,6 +107,10 @@ impl EventHandler {
 }
 
 impl CmapEventHandler for EventHandler {
+    fn handle_pool_created_event(&self, event: PoolCreatedEvent) {
+        self.handle(CmapEvent::ConnectionPoolCreated(event));
+    }
+
     fn handle_pool_cleared_event(&self, event: PoolClearedEvent) {
         self.handle(CmapEvent::ConnectionPoolCleared(event.clone()));
         self.pool_cleared_events.write().unwrap().push_back(event);

src/test/util/matchable.rs

@@ -17,7 +17,7 @@ pub trait Matchable: Sized + 'static {
         if expected.is_placeholder() {
             return true;
         }
-        if let Some(expected) = Any::downcast_ref::<Self>(expected) {
+        if let Some(expected) = <dyn Any>::downcast_ref::<Self>(expected) {
             self.content_matches(expected)
         } else {
             false