fix(dgw): the recording policy wasn't set for RDP via web (#1044)

To prevent future regressions, it is ensured that we properly set the policy everywhere by checking at compile-time that a value is set.
Devolutions · Oct 7, 2024 · 01fb589 · 01fb589
1 parent a42c8b9
commit 01fb589
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 79 deletions.
diff --git a/crates/devolutions-gateway-generators/src/lib.rs b/crates/devolutions-gateway-generators/src/lib.rs
@@ -203,11 +203,14 @@ pub fn any_association_claims(now: i64, validity_duration: i64) -> impl Strategy
 
 pub fn session_info_fwd_only() -> impl Strategy<Value = SessionInfo> {
     (uuid_typed(), application_protocol(), target_addr()).prop_map(|(id, application_protocol, destination_host)| {
-        SessionInfo::new(
-            id,
-            application_protocol,
-            ConnectionModeDetails::Fwd { destination_host },
-        )
+        SessionInfo::builder()
+            .association_id(id)
+            .application_protocol(application_protocol)
+            .details(ConnectionModeDetails::Fwd { destination_host })
+            .recording_policy(token::RecordingPolicy::None)
+            .filtering_policy(false)
+            .time_to_live(token::SessionTtl::Unlimited)
+            .build()
     })
 }
 

diff --git a/devolutions-gateway/src/api/fwd.rs b/devolutions-gateway/src/api/fwd.rs
@@ -181,16 +181,16 @@ where
 
             info!("WebSocket-TLS forwarding");
 
-            let info = SessionInfo::new(
-                claims.jet_aid,
-                claims.jet_ap,
-                ConnectionModeDetails::Fwd {
+            let info = SessionInfo::builder()
+                .association_id(claims.jet_aid)
+                .application_protocol(claims.jet_ap)
+                .details(ConnectionModeDetails::Fwd {
                     destination_host: selected_target.clone(),
-                },
-            )
-            .with_ttl(claims.jet_ttl)
-            .with_recording_policy(claims.jet_rec)
-            .with_filtering_policy(claims.jet_flt);
+                })
+                .time_to_live(claims.jet_ttl)
+                .recording_policy(claims.jet_rec)
+                .filtering_policy(claims.jet_flt)
+                .build();
 
             Proxy::builder()
                 .conf(conf)
@@ -209,16 +209,16 @@ where
         } else {
             info!("WebSocket-TCP forwarding");
 
-            let info = SessionInfo::new(
-                claims.jet_aid,
-                claims.jet_ap,
-                ConnectionModeDetails::Fwd {
+            let info = SessionInfo::builder()
+                .association_id(claims.jet_aid)
+                .application_protocol(claims.jet_ap)
+                .details(ConnectionModeDetails::Fwd {
                     destination_host: selected_target.clone(),
-                },
-            )
-            .with_ttl(claims.jet_ttl)
-            .with_recording_policy(claims.jet_rec)
-            .with_filtering_policy(claims.jet_flt);
+                })
+                .time_to_live(claims.jet_ttl)
+                .recording_policy(claims.jet_rec)
+                .filtering_policy(claims.jet_flt)
+                .build();
 
             Proxy::builder()
                 .conf(conf)

diff --git a/devolutions-gateway/src/generic_client.rs b/devolutions-gateway/src/generic_client.rs
@@ -93,16 +93,16 @@ where
                     .await
                     .context("failed to write leftover bytes")?;
 
-                let info = SessionInfo::new(
-                    claims.jet_aid,
-                    claims.jet_ap,
-                    ConnectionModeDetails::Fwd {
+                let info = SessionInfo::builder()
+                    .association_id(claims.jet_aid)
+                    .application_protocol(claims.jet_ap)
+                    .details(ConnectionModeDetails::Fwd {
                         destination_host: selected_target.clone(),
-                    },
-                )
-                .with_ttl(claims.jet_ttl)
-                .with_recording_policy(claims.jet_rec)
-                .with_filtering_policy(claims.jet_flt);
+                    })
+                    .time_to_live(claims.jet_ttl)
+                    .recording_policy(claims.jet_rec)
+                    .filtering_policy(claims.jet_flt)
+                    .build();
 
                 Proxy::builder()
                     .conf(conf)

diff --git a/devolutions-gateway/src/jmux.rs b/devolutions-gateway/src/jmux.rs
@@ -49,15 +49,15 @@ pub async fn handle(
 
     let session_id = claims.jet_aid;
 
-    let info = SessionInfo::new(
-        session_id,
-        claims.jet_ap,
-        ConnectionModeDetails::Fwd {
+    let info = SessionInfo::builder()
+        .association_id(session_id)
+        .application_protocol(claims.jet_ap)
+        .details(ConnectionModeDetails::Fwd {
             destination_host: main_destination_host,
-        },
-    )
-    .with_ttl(claims.jet_ttl)
-    .with_recording_policy(claims.jet_rec);
+        })
+        .time_to_live(claims.jet_ttl)
+        .recording_policy(claims.jet_rec)
+        .build();
 
     let notify_kill = Arc::new(Notify::new());
 

diff --git a/devolutions-gateway/src/rdp_extension.rs b/devolutions-gateway/src/rdp_extension.rs
@@ -305,14 +305,15 @@ pub async fn handle(
 
     // Start actual RDP session
 
-    let info = SessionInfo::new(
-        claims.jet_aid,
-        claims.jet_ap,
-        ConnectionModeDetails::Fwd {
+    let info = SessionInfo::builder()
+        .association_id(claims.jet_aid)
+        .application_protocol(claims.jet_ap)
+        .details(ConnectionModeDetails::Fwd {
             destination_host: destination.clone(),
-        },
-    )
-    .with_ttl(claims.jet_ttl);
+        })
+        .time_to_live(claims.jet_ttl)
+        .recording_policy(claims.jet_rec)
+        .build();
 
     info!("RDP-TLS forwarding");
 

diff --git a/devolutions-gateway/src/session.rs b/devolutions-gateway/src/session.rs
@@ -15,6 +15,7 @@ use std::time::Duration;
 use tap::prelude::*;
 use time::OffsetDateTime;
 use tokio::sync::{mpsc, oneshot, Notify};
+use typed_builder::TypedBuilder;
 use uuid::Uuid;
 
 #[derive(Debug, Serialize, Clone)]
@@ -25,50 +26,23 @@ pub enum ConnectionModeDetails {
     Fwd { destination_host: TargetAddr },
 }
 
-#[derive(Debug, Serialize, Clone)]
+#[derive(Debug, Serialize, Clone, TypedBuilder)]
 pub struct SessionInfo {
     pub association_id: Uuid,
     pub application_protocol: ApplicationProtocol,
+    #[builder(setter(transform = |value: RecordingPolicy| value != RecordingPolicy::None))]
     pub recording_policy: bool,
+    #[builder(default = false)] // Not enforced yet, so it’s okay to not set it at all for now.
     pub filtering_policy: bool,
+    #[builder(setter(skip), default = OffsetDateTime::now_utc())]
     #[serde(with = "time::serde::rfc3339")]
     pub start_timestamp: OffsetDateTime,
     pub time_to_live: SessionTtl,
     #[serde(flatten)]
-    pub mode_details: ConnectionModeDetails,
+    pub details: ConnectionModeDetails,
 }
 
 impl SessionInfo {
-    pub fn new(association_id: Uuid, ap: ApplicationProtocol, mode_details: ConnectionModeDetails) -> Self {
-        Self {
-            association_id,
-            application_protocol: ap,
-            recording_policy: false,
-            filtering_policy: false,
-            start_timestamp: OffsetDateTime::now_utc(),
-            time_to_live: SessionTtl::Unlimited,
-            mode_details,
-        }
-    }
-
-    #[must_use]
-    pub fn with_recording_policy(mut self, value: RecordingPolicy) -> Self {
-        self.recording_policy = value != RecordingPolicy::None;
-        self
-    }
-
-    #[must_use]
-    pub fn with_filtering_policy(mut self, value: bool) -> Self {
-        self.filtering_policy = value;
-        self
-    }
-
-    #[must_use]
-    pub fn with_ttl(mut self, value: SessionTtl) -> Self {
-        self.time_to_live = value;
-        self
-    }
-
     pub fn id(&self) -> Uuid {
         self.association_id
     }