diff --git a/Cargo.lock b/Cargo.lock index e58dc46e0e5e4..a1409f9df8bc0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1161,7 +1161,7 @@ dependencies = [ [[package]] name = "ironrdp-async" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "bytes", "ironrdp-connector", @@ -1172,7 +1172,7 @@ dependencies = [ [[package]] name = "ironrdp-cliprdr" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "bitflags 2.4.2", "ironrdp-pdu", @@ -1184,7 +1184,7 @@ dependencies = [ [[package]] name = "ironrdp-connector" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "ironrdp-error", "ironrdp-pdu", @@ -1199,12 +1199,12 @@ dependencies = [ [[package]] name = "ironrdp-error" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" [[package]] name = "ironrdp-graphics" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "bit_field", "bitflags 2.4.2", @@ -1221,7 +1221,7 @@ dependencies = [ [[package]] name = "ironrdp-pdu" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "bit_field", "bitflags 2.4.2", @@ -1243,7 +1243,7 @@ dependencies = [ [[package]] name = "ironrdp-rdpdr" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "bitflags 2.4.2", "ironrdp-error", @@ -1255,7 +1255,7 @@ dependencies = [ [[package]] name = "ironrdp-rdpsnd" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "ironrdp-pdu", "ironrdp-svc", @@ -1264,7 +1264,7 @@ dependencies = [ [[package]] name = "ironrdp-session" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "bitflags 2.4.2", "ironrdp-connector", @@ -1278,7 +1278,7 @@ dependencies = [ [[package]] name = "ironrdp-svc" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "bitflags 2.4.2", "ironrdp-pdu", @@ -1287,7 +1287,7 @@ dependencies = [ [[package]] name = "ironrdp-tls" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "tokio", "tokio-rustls", @@ -1297,7 +1297,7 @@ dependencies = [ [[package]] name = "ironrdp-tokio" version = "0.1.0" -source = "git+https://github.com/Devolutions/IronRDP?rev=d24000a2aeab74f2610a0b01c629f7a4ef3c77ef#d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" +source = "git+https://github.com/Devolutions/IronRDP?rev=c944674ecbb0952f9e7ecb964a6c79cdca669a08#c944674ecbb0952f9e7ecb964a6c79cdca669a08" dependencies = [ "bytes", "ironrdp-async", @@ -1523,6 +1523,12 @@ dependencies = [ "zeroize", ] +[[package]] +name = "num-conv" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "51d515d32fb182ee37cda2ccdcb92950d6a3c2893aa280e540671c2cd0f3b1d9" + [[package]] name = "num-derive" version = "0.4.1" @@ -2702,13 +2708,14 @@ dependencies = [ [[package]] name = "time" -version = "0.3.31" +version = "0.3.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f657ba42c3f86e7680e53c8cd3af8abbe56b5491790b46e22e19c0d57463583e" +checksum = "c8248b6521bb14bc45b4067159b9b6ad792e2d6d754d6c41fb50e29fefe38749" dependencies = [ "deranged", "itoa", "js-sys", + "num-conv", "powerfmt", "serde", "time-core", @@ -2723,10 +2730,11 @@ checksum = "ef927ca75afb808a4d64dd374f00a2adf8d0fcff8e7b184af886c3c87ec4a3f3" [[package]] name = "time-macros" -version = "0.2.16" +version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "26197e33420244aeb70c3e8c78376ca46571bc4e701e4791c2cd9f57dcb3a43f" +checksum = "7ba3a3ef41e6672a2f0f001392bb5dcd3ff0a9992d618ca761a11c3121547774" dependencies = [ + "num-conv", "time-core", ] @@ -2747,9 +2755,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.35.1" +version = "1.36.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c89b4efa943be685f629b149f53829423f8f5531ea21249408e8e2f8671ec104" +checksum = "61285f6515fa018fb2d1e46eb21223fff441ee8db5d0f1435e8ab4f5cdb80931" dependencies = [ "backtrace", "bytes", diff --git a/Cargo.toml b/Cargo.toml index e1ee0c6ec551a..c98ec9bd3f260 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,13 +23,13 @@ codegen-units = 1 [workspace.dependencies] # Note: To use a local IronRDP repository as a crate (for example, ironrdp-cliprdr), define the dependency as follows: # ironrdp-cliprdr = { path = "/path/to/local/IronRDP/crates/ironrdp-cliprdr" } -ironrdp-cliprdr = { git = "https://github.com/Devolutions/IronRDP", rev = "d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" } -ironrdp-connector = { git = "https://github.com/Devolutions/IronRDP", rev = "d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" } -ironrdp-graphics = { git = "https://github.com/Devolutions/IronRDP", rev = "d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" } -ironrdp-pdu = { git = "https://github.com/Devolutions/IronRDP", rev = "d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" } -ironrdp-rdpdr = { git = "https://github.com/Devolutions/IronRDP", rev = "d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" } -ironrdp-rdpsnd = { git = "https://github.com/Devolutions/IronRDP", rev = "d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" } -ironrdp-session = { git = "https://github.com/Devolutions/IronRDP", rev = "d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" } -ironrdp-svc = { git = "https://github.com/Devolutions/IronRDP", rev = "d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" } -ironrdp-tls = { git = "https://github.com/Devolutions/IronRDP", rev = "d24000a2aeab74f2610a0b01c629f7a4ef3c77ef", features = ["rustls"]} -ironrdp-tokio = { git = "https://github.com/Devolutions/IronRDP", rev = "d24000a2aeab74f2610a0b01c629f7a4ef3c77ef" } +ironrdp-cliprdr = { git = "https://github.com/Devolutions/IronRDP", rev = "c944674ecbb0952f9e7ecb964a6c79cdca669a08" } +ironrdp-connector = { git = "https://github.com/Devolutions/IronRDP", rev = "c944674ecbb0952f9e7ecb964a6c79cdca669a08" } +ironrdp-graphics = { git = "https://github.com/Devolutions/IronRDP", rev = "c944674ecbb0952f9e7ecb964a6c79cdca669a08" } +ironrdp-pdu = { git = "https://github.com/Devolutions/IronRDP", rev = "c944674ecbb0952f9e7ecb964a6c79cdca669a08" } +ironrdp-rdpdr = { git = "https://github.com/Devolutions/IronRDP", rev = "c944674ecbb0952f9e7ecb964a6c79cdca669a08" } +ironrdp-rdpsnd = { git = "https://github.com/Devolutions/IronRDP", rev = "c944674ecbb0952f9e7ecb964a6c79cdca669a08" } +ironrdp-session = { git = "https://github.com/Devolutions/IronRDP", rev = "c944674ecbb0952f9e7ecb964a6c79cdca669a08" } +ironrdp-svc = { git = "https://github.com/Devolutions/IronRDP", rev = "c944674ecbb0952f9e7ecb964a6c79cdca669a08" } +ironrdp-tls = { git = "https://github.com/Devolutions/IronRDP", rev = "c944674ecbb0952f9e7ecb964a6c79cdca669a08", features = ["rustls"]} +ironrdp-tokio = { git = "https://github.com/Devolutions/IronRDP", rev = "c944674ecbb0952f9e7ecb964a6c79cdca669a08" } diff --git a/lib/srv/desktop/rdp/rdpclient/Cargo.toml b/lib/srv/desktop/rdp/rdpclient/Cargo.toml index 955ab5b0bdc3a..b92e5a731da9d 100644 --- a/lib/srv/desktop/rdp/rdpclient/Cargo.toml +++ b/lib/srv/desktop/rdp/rdpclient/Cargo.toml @@ -32,7 +32,7 @@ rand_chacha = "0.3.1" rsa = "0.9.6" sspi = { git = "https://github.com/Devolutions/sspi-rs", rev="d54bdfcafa0e10d9d78224ebacc4f2a0992a6b79", features = ["network_client"] } static_init = "1.0.3" -tokio = { version = "1.35", features = ["full"] } +tokio = { version = "1.36", features = ["full"] } tokio-boring = { git = "https://github.com/gravitational/boring", rev="605253d99d5e363e178bcf97e1d4622e33844cd5", optional = true } utf16string = "0.2.0" uuid = { version = "1.6.1", features = ["v4"] } diff --git a/lib/srv/desktop/rdp/rdpclient/client.go b/lib/srv/desktop/rdp/rdpclient/client.go index b4fe0d057b6e7..920c8ba3c29e5 100644 --- a/lib/srv/desktop/rdp/rdpclient/client.go +++ b/lib/srv/desktop/rdp/rdpclient/client.go @@ -72,6 +72,7 @@ import "C" import ( "context" + "fmt" "os" "runtime/cgo" "sync" @@ -253,16 +254,24 @@ func (c *Client) readClientSize() error { } if c.requestedWidth > types.MaxRDPScreenWidth || c.requestedHeight > types.MaxRDPScreenHeight { - return trace.BadParameter( + err = trace.BadParameter( "screen size of %d x %d is greater than the maximum allowed by RDP (%d x %d)", s.Width, s.Height, types.MaxRDPScreenWidth, types.MaxRDPScreenHeight, ) + if err := c.sendTDPNotification(err.Error(), tdp.SeverityError); err != nil { + return trace.Wrap(err) + } + return trace.Wrap(err) } return nil } } +func (c *Client) sendTDPNotification(message string, severity tdp.Severity) error { + return c.cfg.Conn.WriteMessage(tdp.Notification{Message: message, Severity: severity}) +} + func (c *Client) startRustRDP(ctx context.Context) error { c.cfg.Log.Info("Rust RDP loop starting") defer c.cfg.Log.Info("Rust RDP loop finished") @@ -292,7 +301,7 @@ func (c *Client) startRustRDP(ctx context.Context) error { return trace.BadParameter("user key was nil") } - if res := C.client_run( + res := C.client_run( C.uintptr_t(c.handle), C.CGOConnectParams{ go_addr: addr, @@ -308,14 +317,37 @@ func (c *Client) startRustRDP(ctx context.Context) error { allow_directory_sharing: C.bool(c.cfg.AllowDirectorySharing), show_desktop_wallpaper: C.bool(c.cfg.ShowDesktopWallpaper), }, - ); res.err_code != C.ErrCodeSuccess { - if res.message == nil { - return trace.Errorf("unknown error: %v", res.err_code) - } + ) + + var message string + if res.message != nil { + message = C.GoString(res.message) defer C.free_string(res.message) - return trace.Errorf("%s", C.GoString(res.message)) } + // If the client exited with an error, send a tdp error notification and return it. + if res.err_code != C.ErrCodeSuccess { + var err error + + if message != "" { + err = trace.Errorf("RDP client exited with an error: %v", message) + } else { + err = trace.Errorf("RDP client exited with an unknown error") + } + + c.sendTDPNotification(err.Error(), tdp.SeverityError) + return err + } + + if message != "" { + message = fmt.Sprintf("RDP client exited gracefully with message: %v", message) + } else { + message = "RDP client exited gracefully" + } + + c.cfg.Log.Info(message) + c.sendTDPNotification(message, tdp.SeverityInfo) + return nil } diff --git a/lib/srv/desktop/rdp/rdpclient/src/client.rs b/lib/srv/desktop/rdp/rdpclient/src/client.rs index 0e42622c237a0..c63c5bf4796e6 100644 --- a/lib/srv/desktop/rdp/rdpclient/src/client.rs +++ b/lib/srv/desktop/rdp/rdpclient/src/client.rs @@ -23,13 +23,14 @@ use crate::{ #[cfg(feature = "fips")] use boring::error::ErrorStack; use bytes::BytesMut; -use ironrdp_cliprdr::{Cliprdr, CliprdrSvcMessages}; +use ironrdp_cliprdr::{Client as ClientRole, Cliprdr, CliprdrSvcMessages}; use ironrdp_connector::{Config, ConnectorError, Credentials}; use ironrdp_pdu::input::fast_path::{ FastPathInput, FastPathInputEvent, KeyboardFlags, SynchronizeFlags, }; use ironrdp_pdu::input::mouse::PointerFlags; use ironrdp_pdu::input::{InputEventError, MousePdu}; +use ironrdp_pdu::mcs::DisconnectReason; use ironrdp_pdu::rdp::capability_sets::MajorPlatformType; use ironrdp_pdu::rdp::RdpError; use ironrdp_pdu::{custom_err, function, PduError, PduParsing}; @@ -37,10 +38,10 @@ use ironrdp_rdpdr::pdu::efs::ClientDeviceListAnnounce; use ironrdp_rdpdr::pdu::RdpdrPdu; use ironrdp_rdpdr::Rdpdr; use ironrdp_rdpsnd::Rdpsnd; -use ironrdp_session::x224::{Processor as X224Processor, Processor}; +use ironrdp_session::x224::{self, ProcessorOutput}; use ironrdp_session::SessionErrorKind::Reason; use ironrdp_session::{reason_err, SessionError, SessionResult}; -use ironrdp_svc::{StaticVirtualChannelProcessor, SvcMessage, SvcProcessorMessages}; +use ironrdp_svc::{SvcMessage, SvcProcessor, SvcProcessorMessages}; use ironrdp_tokio::{Framed, TokioStream}; use log::debug; use rand::{Rng, SeedableRng}; @@ -71,7 +72,7 @@ pub struct Client { read_stream: Option, write_stream: Option, function_receiver: Option, - x224_processor: Arc>, + x224_processor: Arc>, } impl Client { @@ -84,7 +85,10 @@ impl Client { /// /// This function hangs until the RDP session ends or a [`ClientFunction::Stop`] is dispatched /// (see [`ClientHandle::stop`]). - pub fn run(cgo_handle: CgoHandle, params: ConnectParams) -> ClientResult<()> { + pub fn run( + cgo_handle: CgoHandle, + params: ConnectParams, + ) -> ClientResult> { global::TOKIO_RT.block_on(async { Self::connect(cgo_handle, params) .await? @@ -199,7 +203,7 @@ impl Client { let read_stream = ironrdp_tokio::TokioFramed::new(read_stream); let write_stream = ironrdp_tokio::TokioFramed::new(write_stream); - let x224_processor = X224Processor::new( + let x224_processor = x224::Processor::new( connection_result.static_channels, connection_result.user_channel_id, connection_result.io_channel_id, @@ -235,7 +239,7 @@ impl Client { /// which it then executes. /// /// When either loop returns, the other is aborted and the result is returned. - async fn run_loops(mut self) -> ClientResult<()> { + async fn run_loops(mut self) -> ClientResult> { let read_stream = self .read_stream .take() @@ -273,7 +277,11 @@ impl Client { }, res = &mut write_loop_handle => { read_loop_handle.abort(); - res? + match res { + Ok(Ok(())) => Ok(None), + Ok(Err(client_error)) => Err(client_error), + Err(join_error) => Err(join_error.into()), + } } } } @@ -281,9 +289,9 @@ impl Client { fn run_read_loop( cgo_handle: CgoHandle, mut read_stream: RdpReadStream, - x224_processor: Arc>, + x224_processor: Arc>, write_requester: ClientHandle, - ) -> tokio::task::JoinHandle> { + ) -> tokio::task::JoinHandle>> { global::TOKIO_RT.spawn(async move { loop { let (action, mut frame) = read_stream.read_pdu().await?; @@ -305,8 +313,17 @@ impl Client { // X224 PDU, process it and send any immediate response frames to the write loop // for writing to the RDP server. let res = Client::x224_process(x224_processor.clone(), frame).await?; - // Send response frames to write loop for writing to RDP server. - write_requester.write_raw_pdu_async(res).await?; + for output in res { + match output { + ProcessorOutput::ResponseFrame(frame) => { + // Send response frames to write loop for writing to RDP server. + write_requester.write_raw_pdu_async(frame).await?; + } + ProcessorOutput::Disconnect(reason) => { + return Ok(Some(reason)); + } + } + } } } } @@ -317,7 +334,7 @@ impl Client { cgo_handle: CgoHandle, mut write_stream: RdpWriteStream, mut write_receiver: FunctionReceiver, - x224_processor: Arc>, + x224_processor: Arc>, ) -> tokio::task::JoinHandle> { global::TOKIO_RT.spawn(async move { loop { @@ -399,7 +416,7 @@ impl Client { } async fn update_clipboard( - x224_processor: Arc>, + x224_processor: Arc>, data: String, ) -> ClientResult<()> { global::TOKIO_RT @@ -422,15 +439,15 @@ impl Client { } async fn write_cliprdr( - x224_processor: Arc>, + x224_processor: Arc>, write_stream: &mut RdpWriteStream, fun: Box, ) -> ClientResult<()> { let processor = x224_processor.clone(); - let messages: ClientResult = global::TOKIO_RT + let messages: ClientResult> = global::TOKIO_RT .spawn_blocking(move || { let mut x224_processor = Self::x224_lock(&processor)?; - let cliprdr = Self::get_svc_processor::(&mut x224_processor)?; + let cliprdr = Self::get_svc_processor::>(&mut x224_processor)?; Ok(fun.call(cliprdr)?) }) .await?; @@ -447,6 +464,11 @@ impl Client { if !key.down { flags = KeyboardFlags::RELEASE; } + let extended = key.code & 0xE000 == 0xE000; + if extended { + flags |= KeyboardFlags::EXTENDED; + } + let event = FastPathInputEvent::KeyboardEvent(flags, key.code as u8); Self::write_fast_path_input_event(write_stream, event).await @@ -534,7 +556,7 @@ impl Client { async fn write_rdpdr( write_stream: &mut RdpWriteStream, - x224_processor: Arc>, + x224_processor: Arc>, pdu: RdpdrPdu, ) -> ClientResult<()> { debug!("sending rdp: {:?}", pdu); @@ -552,7 +574,7 @@ impl Client { async fn handle_tdp_sd_announce( write_stream: &mut RdpWriteStream, - x224_processor: Arc>, + x224_processor: Arc>, sda: tdp::SharedDirectoryAnnounce, ) -> ClientResult<()> { debug!("received tdp: {:?}", sda); @@ -567,7 +589,7 @@ impl Client { } async fn handle_tdp_sd_info_response( - x224_processor: Arc>, + x224_processor: Arc>, res: tdp::SharedDirectoryInfoResponse, ) -> ClientResult<()> { global::TOKIO_RT @@ -582,7 +604,7 @@ impl Client { } async fn handle_tdp_sd_create_response( - x224_processor: Arc>, + x224_processor: Arc>, res: tdp::SharedDirectoryCreateResponse, ) -> ClientResult<()> { global::TOKIO_RT @@ -597,7 +619,7 @@ impl Client { } async fn handle_tdp_sd_delete_response( - x224_processor: Arc>, + x224_processor: Arc>, res: tdp::SharedDirectoryDeleteResponse, ) -> ClientResult<()> { global::TOKIO_RT @@ -612,7 +634,7 @@ impl Client { } async fn handle_tdp_sd_list_response( - x224_processor: Arc>, + x224_processor: Arc>, res: tdp::SharedDirectoryListResponse, ) -> ClientResult<()> { global::TOKIO_RT @@ -627,7 +649,7 @@ impl Client { } async fn handle_tdp_sd_read_response( - x224_processor: Arc>, + x224_processor: Arc>, res: tdp::SharedDirectoryReadResponse, ) -> ClientResult<()> { global::TOKIO_RT @@ -642,7 +664,7 @@ impl Client { } async fn handle_tdp_sd_write_response( - x224_processor: Arc>, + x224_processor: Arc>, res: tdp::SharedDirectoryWriteResponse, ) -> ClientResult<()> { global::TOKIO_RT @@ -657,7 +679,7 @@ impl Client { } async fn handle_tdp_sd_move_response( - x224_processor: Arc>, + x224_processor: Arc>, res: tdp::SharedDirectoryMoveResponse, ) -> ClientResult<()> { global::TOKIO_RT @@ -672,7 +694,7 @@ impl Client { } async fn add_drive( - x224_processor: Arc>, + x224_processor: Arc>, sda: tdp::SharedDirectoryAnnounce, ) -> ClientResult { global::TOKIO_RT @@ -692,9 +714,9 @@ impl Client { /// This function ensures `x224_processor` is locked only for the necessary duration /// of the function call. async fn x224_process( - x224_processor: Arc>, + x224_processor: Arc>, frame: BytesMut, - ) -> SessionResult> { + ) -> SessionResult> { global::TOKIO_RT .spawn_blocking(move || Self::x224_lock(&x224_processor)?.process(&frame)) .await @@ -707,8 +729,8 @@ impl Client { /// while waiting for the `x224_processor` lock, or while processing the frame. /// This function ensures `x224_processor` is locked only for the necessary duration /// of the function call. - async fn x224_process_svc_messages( - x224_processor: Arc>, + async fn x224_process_svc_messages( + x224_processor: Arc>, messages: SvcProcessorMessages, ) -> SessionResult> { global::TOKIO_RT @@ -720,14 +742,14 @@ impl Client { } fn x224_lock( - x224_processor: &Arc>, - ) -> Result, SessionError> { + x224_processor: &Arc>, + ) -> Result, SessionError> { x224_processor .lock() .map_err(|err| reason_err!(function!(), "PoisonError: {:?}", err)) } - /// Returns an immutable reference to the [`StaticVirtualChannelProcessor`] of type `S`. + /// Returns an immutable reference to the [`SvcProcessor`] of type `S`. /// /// # Example /// @@ -737,10 +759,10 @@ impl Client { /// // Now we can call methods on the Cliprdr processor. /// ``` fn get_svc_processor<'a, S>( - x224_processor: &'a mut MutexGuard<'_, X224Processor>, + x224_processor: &'a mut MutexGuard<'_, x224::Processor>, ) -> Result<&'a S, ClientError> where - S: StaticVirtualChannelProcessor + 'static, + S: SvcProcessor + 'static, { x224_processor .get_svc_processor::() @@ -750,7 +772,7 @@ impl Client { ))) } - /// Returns a mutable reference to the [`StaticVirtualChannelProcessor`] of type `S`. + /// Returns a mutable reference to the [`SvcProcessor`] of type `S`. /// /// # Example /// @@ -760,10 +782,10 @@ impl Client { /// // Now we can call mutating methods on the Cliprdr processor. /// ``` fn get_svc_processor_mut<'a, S>( - x224_processor: &'a mut MutexGuard<'_, X224Processor>, + x224_processor: &'a mut MutexGuard<'_, x224::Processor>, ) -> Result<&'a mut S, ClientError> where - S: StaticVirtualChannelProcessor + 'static, + S: SvcProcessor + 'static, { x224_processor .get_svc_processor_mut::() @@ -775,10 +797,10 @@ impl Client { /// Returns a mutable reference to the [`TeleportCliprdrBackend`] of the [`Cliprdr`] processor. fn cliprdr_backend( - x224_processor: &mut X224Processor, + x224_processor: &mut x224::Processor, ) -> ClientResult<&mut TeleportCliprdrBackend> { x224_processor - .get_svc_processor_mut::() + .get_svc_processor_mut::>() .and_then(|c| c.downcast_backend_mut::()) .ok_or(ClientError::InternalError( "cliprdr_backend returned None".to_string(), @@ -787,7 +809,7 @@ impl Client { /// Returns a mutable reference to the [`TeleportRdpdrBackend`] of the [`Rdpdr`] processor. fn rdpdr_backend( - x224_processor: &mut X224Processor, + x224_processor: &mut x224::Processor, ) -> ClientResult<&mut TeleportRdpdrBackend> { x224_processor .get_svc_processor_mut::() diff --git a/lib/srv/desktop/rdp/rdpclient/src/cliprdr.rs b/lib/srv/desktop/rdp/rdpclient/src/cliprdr.rs index 8cb7a875adc05..62912d8d9b714 100644 --- a/lib/srv/desktop/rdp/rdpclient/src/cliprdr.rs +++ b/lib/srv/desktop/rdp/rdpclient/src/cliprdr.rs @@ -21,7 +21,7 @@ use ironrdp_cliprdr::pdu::{ ClipboardFormat, ClipboardFormatId, ClipboardGeneralCapabilityFlags, FileContentsRequest, FileContentsResponse, FormatDataRequest, FormatDataResponse, LockDataId, }; -use ironrdp_cliprdr::{Cliprdr, CliprdrSvcMessages}; +use ironrdp_cliprdr::{Client as ClientRole, Cliprdr, CliprdrSvcMessages}; use ironrdp_pdu::PduResult; use ironrdp_svc::impl_as_any; use log::{debug, error, info, trace, warn}; @@ -57,7 +57,7 @@ impl TeleportCliprdrBackend { fn send(&self, name: &'static str, f: F) where - F: Fn(&Cliprdr) -> PduResult + Send + 'static, + F: Fn(&Cliprdr) -> PduResult> + Send + 'static, { let res = self .client_handle @@ -189,21 +189,24 @@ impl CliprdrBackend for TeleportCliprdrBackend { impl_as_any!(TeleportCliprdrBackend); pub trait ClipboardFn: Send + Debug + 'static { - fn call(&self, cliprdr: &Cliprdr) -> PduResult; + fn call(&self, cliprdr: &Cliprdr) -> PduResult>; } impl ClipboardFn for F where - F: Fn(&Cliprdr) -> PduResult + Send + Debug + 'static, + F: Fn(&Cliprdr) -> PduResult> + + Send + + Debug + + 'static, { - fn call(&self, cliprdr: &Cliprdr) -> PduResult { + fn call(&self, cliprdr: &Cliprdr) -> PduResult> { (self)(cliprdr) } } struct ClipboardFnInternal where - F: Fn(&Cliprdr) -> PduResult + Send + 'static, + F: Fn(&Cliprdr) -> PduResult> + Send + 'static, { name: &'static str, closure: F, @@ -211,7 +214,7 @@ where impl ClipboardFnInternal where - F: Fn(&Cliprdr) -> PduResult + Send + 'static, + F: Fn(&Cliprdr) -> PduResult> + Send + 'static, { fn new(name: &'static str, closure: F) -> Self { Self { name, closure } @@ -220,7 +223,7 @@ where impl Debug for ClipboardFnInternal where - F: Fn(&Cliprdr) -> PduResult + Send + 'static, + F: Fn(&Cliprdr) -> PduResult> + Send + 'static, { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!(f, "{}", &self.name) @@ -229,9 +232,9 @@ where impl ClipboardFn for ClipboardFnInternal where - F: Fn(&Cliprdr) -> PduResult + Send + 'static, + F: Fn(&Cliprdr) -> PduResult> + Send + 'static, { - fn call(&self, cliprdr: &Cliprdr) -> PduResult { + fn call(&self, cliprdr: &Cliprdr) -> PduResult> { (self.closure)(cliprdr) } } diff --git a/lib/srv/desktop/rdp/rdpclient/src/lib.rs b/lib/srv/desktop/rdp/rdpclient/src/lib.rs index 8edd175d739eb..8203f03150c26 100644 --- a/lib/srv/desktop/rdp/rdpclient/src/lib.rs +++ b/lib/srv/desktop/rdp/rdpclient/src/lib.rs @@ -103,10 +103,16 @@ pub unsafe extern "C" fn client_run(cgo_handle: CgoHandle, params: CGOConnectPar show_desktop_wallpaper: params.show_desktop_wallpaper, }, ) { - Ok(_) => CGOResult { + Ok(res) => CGOResult { err_code: CGOErrCode::ErrCodeSuccess, - message: ptr::null_mut(), + message: match res { + Some(reason) => CString::new(reason.description().to_string()) + .map(|c| c.into_raw()) + .unwrap_or(ptr::null_mut()), + None => ptr::null_mut(), + }, }, + Err(e) => { error!("client_run failed: {:?}", e); CGOResult { diff --git a/web/packages/teleport/src/DesktopSession/DesktopSession.story.test.tsx b/web/packages/teleport/src/DesktopSession/DesktopSession.story.test.tsx index ae3f6cfc3f05f..5ee242630efd5 100644 --- a/web/packages/teleport/src/DesktopSession/DesktopSession.story.test.tsx +++ b/web/packages/teleport/src/DesktopSession/DesktopSession.story.test.tsx @@ -21,21 +21,21 @@ import 'jest-canvas-mock'; import { render, screen } from 'design/utils/testing'; import { - Processing, + BothProcessing, TdpProcessing, - InvalidProcessingState, + FetchProcessing, ConnectedSettingsFalse, ConnectedSettingsTrue, Disconnected, FetchError, - ConnectionError, + TdpError, UnintendedDisconnect, WebAuthnPrompt, AnotherSessionActive, } from './DesktopSession.story'; test('processing', () => { - const { container } = render(); + const { container } = render(); expect(container).toMatchSnapshot(); }); @@ -44,9 +44,9 @@ test('tdp processing', () => { expect(container).toMatchSnapshot(); }); -test('invalid processing', () => { - render(); - expect(screen.getByTestId('Modal')).toMatchSnapshot(); +test('fetch processing', () => { + const { container } = render(); + expect(container).toMatchSnapshot(); }); test('connected settings false', () => { @@ -70,7 +70,7 @@ test('fetch error', () => { }); test('connection error', () => { - render(); + render(); expect(screen.getByTestId('Modal')).toMatchSnapshot(); }); diff --git a/web/packages/teleport/src/DesktopSession/DesktopSession.story.tsx b/web/packages/teleport/src/DesktopSession/DesktopSession.story.tsx index f597fb04b63ec..f3980b70ce034 100644 --- a/web/packages/teleport/src/DesktopSession/DesktopSession.story.tsx +++ b/web/packages/teleport/src/DesktopSession/DesktopSession.story.tsx @@ -53,9 +53,7 @@ const props: State = { username: 'user', clientOnWsOpen: () => {}, clientOnWsClose: () => {}, - wsConnection: 'closed', - disconnected: false, - setDisconnected: () => {}, + wsConnection: { status: 'closed', statusText: 'websocket closed' }, setClipboardSharingState: () => {}, directorySharingState: { allowedByAcl: true, @@ -69,6 +67,7 @@ const props: State = { clientOnClientScreenSpec: () => {}, clientScreenSpecToRequest: { width: 0, height: 0 }, clientOnTdpError: () => {}, + clientOnTdpInfo: () => {}, clientOnTdpWarning: () => {}, canvasOnKeyDown: () => {}, canvasOnKeyUp: () => {}, @@ -93,7 +92,7 @@ const props: State = { onRemoveWarning: () => {}, }; -export const Processing = () => ( +export const BothProcessing = () => ( ( readState: 'granted', writeState: 'granted', }} - wsConnection={'open'} - disconnected={false} + wsConnection={{ status: 'open' }} /> ); @@ -120,12 +118,11 @@ export const TdpProcessing = () => ( readState: 'granted', writeState: 'granted', }} - wsConnection={'open'} - disconnected={false} + wsConnection={{ status: 'open' }} /> ); -export const InvalidProcessingState = () => ( +export const FetchProcessing = () => ( ( readState: 'granted', writeState: 'granted', }} - wsConnection={'open'} - disconnected={false} + wsConnection={{ status: 'open' }} + /> +); + +export const FetchError = () => ( + +); + +export const TdpError = () => ( + +); + +export const TdpGraceful = () => ( + ); @@ -153,13 +182,17 @@ export const ConnectedSettingsFalse = () => { tdpClient={client} fetchAttempt={{ status: 'success' }} tdpConnection={{ status: 'success' }} - wsConnection={'open'} - disconnected={false} + wsConnection={{ status: 'open' }} clipboardSharingState={{ - allowedByAcl: true, - browserSupported: true, - readState: 'granted', - writeState: 'granted', + allowedByAcl: false, + browserSupported: false, + readState: 'denied', + writeState: 'denied', + }} + directorySharingState={{ + allowedByAcl: false, + browserSupported: false, + directorySelected: false, }} clientOnPngFrame={(ctx: CanvasRenderingContext2D) => { fillGray(ctx.canvas); @@ -180,8 +213,7 @@ export const ConnectedSettingsTrue = () => { tdpClient={client} fetchAttempt={{ status: 'success' }} tdpConnection={{ status: 'success' }} - wsConnection={'open'} - disconnected={false} + wsConnection={{ status: 'open' }} clipboardSharingState={{ allowedByAcl: true, browserSupported: true, @@ -205,31 +237,7 @@ export const Disconnected = () => ( {...props} fetchAttempt={{ status: 'success' }} tdpConnection={{ status: 'success' }} - wsConnection={'open'} - disconnected={true} - /> -); - -export const FetchError = () => ( - -); - -export const ConnectionError = () => ( - ); @@ -238,8 +246,7 @@ export const UnintendedDisconnect = () => ( {...props} fetchAttempt={{ status: 'success' }} tdpConnection={{ status: 'success' }} - disconnected={false} - wsConnection={'closed'} + wsConnection={{ status: 'closed' }} /> ); @@ -254,8 +261,7 @@ export const WebAuthnPrompt = () => ( readState: 'granted', writeState: 'granted', }} - wsConnection={'open'} - disconnected={false} + wsConnection={{ status: 'open' }} webauthn={{ errorText: '', requested: true, @@ -304,8 +310,7 @@ export const Warnings = () => { tdpClient={client} fetchAttempt={{ status: 'success' }} tdpConnection={{ status: 'success' }} - wsConnection={'open'} - disconnected={false} + wsConnection={{ status: 'open' }} clipboardSharingState={{ allowedByAcl: true, browserSupported: true, diff --git a/web/packages/teleport/src/DesktopSession/DesktopSession.tsx b/web/packages/teleport/src/DesktopSession/DesktopSession.tsx index 119b601b59eb4..0c1259064277d 100644 --- a/web/packages/teleport/src/DesktopSession/DesktopSession.tsx +++ b/web/packages/teleport/src/DesktopSession/DesktopSession.tsx @@ -16,22 +16,16 @@ * along with this program. If not, see . */ -import React from 'react'; -import { - Indicator, - Box, - Text, - Flex, - ButtonSecondary, - ButtonPrimary, -} from 'design'; -import { Danger, Info } from 'design/Alert'; +import React, { useState, useEffect } from 'react'; +import { Indicator, Box, Flex, ButtonSecondary, ButtonPrimary } from 'design'; +import { Info } from 'design/Alert'; import Dialog, { DialogHeader, DialogTitle, DialogContent, DialogFooter, } from 'design/Dialog'; +import { Attempt } from 'shared/hooks/useAttemptNext'; import TdpClientCanvas from 'teleport/components/TdpClientCanvas'; import AuthnDialog from 'teleport/components/AuthnDialog'; @@ -43,9 +37,8 @@ import useDesktopSession, { } from './useDesktopSession'; import TopBar from './TopBar'; -import type { PropsWithChildren } from 'react'; - -import type { State } from './useDesktopSession'; +import type { State, WebsocketAttempt } from './useDesktopSession'; +import type { WebAuthnState } from 'teleport/lib/useWebAuthn'; export default function Container() { const state = useDesktopSession(); @@ -60,225 +53,70 @@ declare global { export function DesktopSession(props: State) { const { + webauthn, + tdpClient, + username, + hostname, + setClipboardSharingState, + directorySharingState, + setDirectorySharingState, + clientOnPngFrame, + clientOnBitmapFrame, + clientOnClientScreenSpec, + clientOnClipboardData, + clientOnTdpError, + clientOnTdpWarning, + clientOnTdpInfo, + clientOnWsClose, + clientOnWsOpen, + canvasOnKeyDown, + canvasOnKeyUp, + canvasOnFocusOut, + canvasOnMouseMove, + canvasOnMouseDown, + canvasOnMouseUp, + canvasOnMouseWheelScroll, + canvasOnContextMenu, + clientScreenSpecToRequest, + clipboardSharingState, + onShareDirectory, + warnings, + onRemoveWarning, fetchAttempt, tdpConnection, - disconnected, wsConnection, - setTdpConnection, showAnotherSessionActiveDialog, - setShowAnotherSessionActiveDialog, } = props; - const processing = - fetchAttempt.status === 'processing' || - tdpConnection.status === 'processing'; - - // onDialogClose is called when a user - // dismisses a non-fatal error dialog. - const onDialogClose = () => { - // The following state-setting calls will - // cause the useEffect below to calculate the - // errorDialog state. - - setTdpConnection(prevState => { - if (prevState.status === '') { - // If prevState.status was a non-fatal error, - // we assume that the TDP connection remains open. - return { status: 'success' }; - } - return prevState; - }); - }; - - const computeErrorDialog = () => { - // Websocket is closed but we haven't - // closed it on purpose or registered a fatal TDP error. - const unknownConnectionError = - wsConnection === 'closed' && - !disconnected && - (tdpConnection.status === 'success' || tdpConnection.status === ''); - - let errorText = ''; - if (fetchAttempt.status === 'failed') { - errorText = fetchAttempt.statusText || 'fetch attempt failed'; - } else if (tdpConnection.status === 'failed') { - errorText = tdpConnection.statusText || 'TDP connection failed'; - } else if (tdpConnection.status === '') { - errorText = tdpConnection.statusText || 'encountered a non-fatal error'; - } else if (unknownConnectionError) { - errorText = 'Session disconnected for an unknown reason.'; - } else if ( - fetchAttempt.status === 'processing' && - tdpConnection.status === 'success' - ) { - errorText = - 'The application has detected an invalid internal application state. \ - Please file a bug report for this issue at \ - https://github.com/gravitational/teleport/issues/new?assignees=&labels=bug&template=bug_report.md'; - } - const open = errorText !== ''; - - return { - open, - text: errorText, - isError: unknownConnectionError || errorText === 'RDP connection failed', - }; - }; - - const errorDialog = computeErrorDialog(); - const Alert = errorDialog.isError ? Danger : Info; - - if (errorDialog.open) { - return ( - - ({ width: '484px' })} - onClose={onDialogClose} - open={errorDialog.open} - > - - - {errorDialog.isError ? 'Error' : 'Disconnected'} - - - - <> - {errorDialog.text}} /> - Refresh the page to reconnect. - - - - { - window.location.reload(); - }} - > - Refresh - - - - - ); - } - - if (showAnotherSessionActiveDialog) { - // Don't start the TDP connection until the user confirms they're ok - // with potentially killing another user's connection. - const shouldConnect = false; - - return ( - - ({ width: '484px' })} - onClose={() => {}} - open={true} - > - - Another Session Is Active - - - This desktop has an active session, connecting to it may close the - other session. Do you wish to continue? - - - { - window.close(); - }} - > - Abort - - { - setShowAnotherSessionActiveDialog(false); - }} - > - Continue - - - - - ); - } + const [screenState, setScreenState] = useState({ + screen: 'processing', + canvasState: { shouldConnect: false, shouldDisplay: false }, + }); - if (disconnected) { - return ( - - - Session successfully disconnected - - + // Calculate the next `ScreenState` whenever any of the constituent pieces of state change. + useEffect(() => { + setScreenState(prevState => + nextScreenState( + prevState, + fetchAttempt, + tdpConnection, + wsConnection, + showAnotherSessionActiveDialog, + webauthn + ) ); - } - - if (processing) { - // We don't know whether another session for this desktop is active while the - // fetchAttempt is still processing, so hold off on starting a TDP connection - // until that information is available. - const shouldConnect = fetchAttempt.status !== 'processing'; - - return ( - - - - - - ); - } - - return ; -} + }, [ + fetchAttempt, + tdpConnection, + wsConnection, + showAnotherSessionActiveDialog, + webauthn, + ]); -function Session({ - setDisconnected, - webauthn, - tdpClient, - username, - hostname, - setClipboardSharingState, - directorySharingState, - setDirectorySharingState, - clientOnPngFrame, - clientOnBitmapFrame, - clientOnClientScreenSpec, - clientOnClipboardData, - clientOnTdpError, - clientOnTdpWarning, - clientOnWsClose, - clientOnWsOpen, - canvasOnKeyDown, - canvasOnKeyUp, - canvasOnFocusOut, - canvasOnMouseMove, - canvasOnMouseDown, - canvasOnMouseUp, - canvasOnMouseWheelScroll, - canvasOnContextMenu, - clientShouldConnect, - clientScreenSpecToRequest, - displayCanvas, - clipboardSharingState, - onShareDirectory, - warnings, - onRemoveWarning, - children, -}: PropsWithChildren) { return ( { - setDisconnected(true); setClipboardSharingState(prevState => ({ ...prevState, isSharing: false, @@ -298,30 +136,21 @@ function Session({ onRemoveWarning={onRemoveWarning} /> - {children} - - {webauthn.requested && ( - { - webauthn.setState(prevState => { - return { - ...prevState, - errorText: - 'This session requires multi factor authentication to continue. Please hit "Retry" and follow the prompts given by your browser to complete authentication.', - }; - }); - }} - errorText={webauthn.errorText} - /> + {screenState.screen === 'anotherSessionActive' && ( + + )} + {screenState.screen === 'mfa' && } + {screenState.screen === 'alert dialog' && ( + )} + {screenState.screen === 'processing' && } { + return ( + { + webauthn.setState(prevState => { + return { + ...prevState, + errorText: + 'This session requires multi factor authentication to continue. Please hit "Retry" and follow the prompts given by your browser to complete authentication.', + }; + }); + }} + errorText={webauthn.errorText} + /> + ); +}; + +const AlertDialog = ({ screenState }: { screenState: ScreenState }) => ( + ({ width: '484px' })} open={true}> + + Disconnected + + + <> + {screenState.alertMessage || invalidStateMessage}} + /> + Refresh the page to reconnect. + + + + { + window.location.reload(); + }} + > + Refresh + + + +); + +const AnotherSessionActiveDialog = (props: State) => { + return ( + ({ width: '484px' })} + onClose={() => {}} + open={true} + > + + Another Session Is Active + + + This desktop has an active session, connecting to it may close the other + session. Do you wish to continue? + + + { + window.close(); + }} + > + Abort + + { + props.setShowAnotherSessionActiveDialog(false); + }} + > + Continue + + + + ); +}; + +const Processing = () => { + return ( + + + + ); +}; + +const invalidStateMessage = 'internal application error'; + +/** + * Calculate the next `ScreenState` based on the current state and the latest + * attempts to fetch the desktop session, connect to the TDP server, and connect + * to the websocket. + */ +const nextScreenState = ( + prevState: ScreenState, + fetchAttempt: Attempt, + tdpConnection: Attempt, + wsConnection: WebsocketAttempt, + showAnotherSessionActiveDialog: boolean, + webauthn: WebAuthnState +): ScreenState => { + // We always want to show the user the first alert that caused the session to fail/end, + // so if we're already showing an alert, don't change the screen. + // + // This allows us to track the various pieces of the state independently and always display + // the vital information to the user. For example, we can track the TDP connection status + // and the websocket connection status separately throughout the codebase. If the TDP connection + // fails, and then the websocket closes, we want to show the `tdpConnection.statusText` to the user, + // not the `wsConnection.statusText`. But if the websocket closes unexpectedly before a TDP message telling + // us why, we want to show the websocket closing message to the user. + if (prevState.screen === 'alert dialog') { + return prevState; + } + + // Otherwise, calculate a new screen state. + const showAnotherSessionActive = showAnotherSessionActiveDialog; + const showMfa = webauthn.requested; + const showAlert = + fetchAttempt.status === 'failed' || // Fetch attempt failed + tdpConnection.status === 'failed' || // TDP connection failed + tdpConnection.status === '' || // TDP connection ended gracefully server-side + wsConnection.status === 'closed'; // Websocket closed (could mean client side graceful close or unexpected close, the message will tell us which) + + const atLeastOneAttemptProcessing = + fetchAttempt.status === 'processing' || + tdpConnection.status === 'processing'; + const noDialogs = !(showMfa || showAnotherSessionActive || showAlert); + const showProcessing = atLeastOneAttemptProcessing && noDialogs; + + if (showAnotherSessionActive) { + // Highest priority: we don't want to connect (`shouldConnect`) until + // the user has decided whether to continue with the active session. + return { + screen: 'anotherSessionActive', + canvasState: { shouldConnect: false, shouldDisplay: false }, + }; + } else if (showMfa) { + // Second highest priority. Secondary to `showAnotherSessionActive` because + // this won't happen until the user has decided whether to continue with the active session. + // + // `shouldConnect` is true because we want to maintain the websocket connection that the mfa + // request was made over. + return { + screen: 'mfa', + canvasState: { shouldConnect: true, shouldDisplay: false }, + }; + } else if (showAlert) { + // Third highest priority. If either attempt or the websocket has failed, show the alert. + return { + screen: 'alert dialog', + alertMessage: calculateAlertMessage( + fetchAttempt, + tdpConnection, + wsConnection, + showAnotherSessionActiveDialog, + prevState + ), + canvasState: { shouldConnect: false, shouldDisplay: false }, + }; + } else if (showProcessing) { + // Fourth highest priority. If at least one attempt is still processing, show the processing indicator + // while trying to connect to the TDP server via the websocket. + const shouldConnect = fetchAttempt.status !== 'processing'; + return { + screen: 'processing', + canvasState: { shouldConnect, shouldDisplay: false }, + }; + } else { + // Default state: everything is good, so show the canvas. + return { + screen: 'canvas', + canvasState: { shouldConnect: true, shouldDisplay: true }, + }; + } +}; + +/** + * Calculate the error message to display to the user based on the current state. + */ +/* eslint-disable no-console */ +const calculateAlertMessage = ( + fetchAttempt: Attempt, + tdpConnection: Attempt, + wsConnection: WebsocketAttempt, + showAnotherSessionActiveDialog: boolean, + prevState: ScreenState +): string => { + let message = ''; + if (fetchAttempt.status === 'failed') { + message = fetchAttempt.statusText || 'fetch attempt failed'; + } else if (tdpConnection.status === 'failed') { + message = tdpConnection.statusText || 'TDP connection failed'; + } else if (tdpConnection.status === '') { + message = tdpConnection.statusText || 'TDP connection ended gracefully'; + } else if (wsConnection.status === 'closed') { + message = + wsConnection.statusText || 'websocket disconnected for an unknown reason'; + } else { + console.error('invalid state'); + console.error({ + fetchAttempt, + tdpConnection, + wsConnection, + showAnotherSessionActiveDialog, + prevState, + }); + message = invalidStateMessage; + } + return message; +}; +/* eslint-enable no-console */ + +type ScreenState = { + screen: + | 'mfa' + | 'anotherSessionActive' + | 'alert dialog' + | 'processing' + | 'canvas'; + + alertMessage?: string; + canvasState: { + shouldConnect: boolean; + shouldDisplay: boolean; + }; }; diff --git a/web/packages/teleport/src/DesktopSession/KeyboardHandler.tsx b/web/packages/teleport/src/DesktopSession/KeyboardHandler.tsx new file mode 100644 index 0000000000000..a7bb98061e901 --- /dev/null +++ b/web/packages/teleport/src/DesktopSession/KeyboardHandler.tsx @@ -0,0 +1,135 @@ +/** + * Teleport + * Copyright (C) 2023 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { Platform, getPlatform } from 'design/platform'; + +import { TdpClient, ButtonState } from 'teleport/lib/tdp'; +import { SyncKeys } from 'teleport/lib/tdp/codec'; + +import { Withholder } from './Withholder'; + +/** + * Handles keyboard events. + */ +export class KeyboardHandler { + private withholder: Withholder = new Withholder(); + /** + * Tracks whether the next keydown or keyup event should sync the + * local toggle key state to the remote machine. + * + * Set to true: + * - On component initialization, so keys are synced before the first keydown/keyup event. + * - On focusout, so keys are synced when the user returns to the window. + */ + private syncBeforeNextKey: boolean = true; + private static isMac: boolean = getPlatform() === Platform.macOS; + + constructor() { + // Bind finishHandlingKeyboardEvent to this instance so it can be passed + // as a callback to the Withholder. + this.finishHandlingKeyboardEvent = + this.finishHandlingKeyboardEvent.bind(this); + } + + /** + * Primary method for handling keyboard events. + */ + public handleKeyboardEvent(params: KeyboardEventParams) { + const { e, cli } = params; + e.preventDefault(); + this.handleSyncBeforeNextKey(cli, e); + this.withholder.handleKeyboardEvent( + params, + this.finishHandlingKeyboardEvent + ); + } + + private handleSyncBeforeNextKey(cli: TdpClient, e: KeyboardEvent) { + if (this.syncBeforeNextKey === true) { + cli.sendSyncKeys(this.getSyncKeys(e)); + this.syncBeforeNextKey = false; + } + } + + private getSyncKeys = (e: KeyboardEvent): SyncKeys => { + return { + scrollLockState: this.getModifierState(e, 'ScrollLock'), + numLockState: this.getModifierState(e, 'NumLock'), + capsLockState: this.getModifierState(e, 'CapsLock'), + kanaLockState: ButtonState.UP, // KanaLock is not supported, see https://www.w3.org/TR/uievents-key/#keys-modifier + }; + }; + + /** + * Returns the ButtonState corresponding to the given `keyArg`. + * + * @param e The `KeyboardEvent` + * @param keyArg The key to check the state of. Valid values can be found [here](https://www.w3.org/TR/uievents-key/#keys-modifier) + */ + private getModifierState = ( + e: KeyboardEvent, + keyArg: string + ): ButtonState => { + return e.getModifierState(keyArg) ? ButtonState.DOWN : ButtonState.UP; + }; + + /** + * Called to finish handling a keyboard event. + * + * For normal keys, this is called immediately. + * For withheld or delayed keys, this is called as the callback when + * another key is pressed or released (withheld) or after a delay (delayed). + */ + private finishHandlingKeyboardEvent(params: KeyboardEventParams): void { + const { cli, e, state } = params; + // Special handling for CapsLock on Mac. + if (e.code === 'CapsLock' && KeyboardHandler.isMac) { + // On Mac, every UP or DOWN given to us by the browser corresponds + // to a DOWN + UP on the remote machine for CapsLock. + cli.sendKeyboardInput('CapsLock', ButtonState.DOWN); + cli.sendKeyboardInput('CapsLock', ButtonState.UP); + } else { + // Otherwise, just pass the event through normally to the server. + cli.sendKeyboardInput(e.code, state); + } + } + + /** + * Must be called when the element associated with the KeyboardHandler loses focus. + */ + public onFocusOut() { + // Sync toggle keys when we come back into focus. + this.syncBeforeNextKey = true; + // Cancel any withheld keys. + this.withholder.cancel(); + } + + /** + * Should be called when the element associated with the KeyboardHandler goes away. + */ + public dispose() { + // Make sure we cancel any withheld keys, particularly we want to cancel the timeouts. + this.withholder.cancel(); + } +} + +export type KeyboardEventParams = { + cli: TdpClient; + e: KeyboardEvent; + state: ButtonState; +}; diff --git a/web/packages/teleport/src/DesktopSession/Withholder.test.ts b/web/packages/teleport/src/DesktopSession/Withholder.test.ts new file mode 100644 index 0000000000000..149748e4c8180 --- /dev/null +++ b/web/packages/teleport/src/DesktopSession/Withholder.test.ts @@ -0,0 +1,133 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { ButtonState, TdpClient } from 'teleport/lib/tdp'; + +import { Withholder } from './Withholder'; + +// Mock the TdpClient class +jest.mock('teleport/lib/tdp', () => { + const originalModule = jest.requireActual('teleport/lib/tdp'); // Get the actual module + return { + ...originalModule, + TdpClient: jest.fn().mockImplementation(() => { + return { + connect: jest.fn().mockResolvedValue(undefined), + }; + }), + }; +}); + +describe('withholder', () => { + jest.useFakeTimers(); + let withholder: Withholder; + let mockHandleKeyboardEvent: jest.Mock; + + beforeEach(() => { + withholder = new Withholder(); + mockHandleKeyboardEvent = jest.fn(); + }); + + afterEach(() => { + withholder.cancel(); + mockHandleKeyboardEvent.mockClear(); + jest.clearAllTimers(); + }); + + it('handles non-withheld keys immediately', () => { + const params = { + e: { key: 'Enter' } as KeyboardEvent as KeyboardEvent, + state: ButtonState.DOWN, + cli: new TdpClient('wss://socketAddr.gov'), + }; + withholder.handleKeyboardEvent(params, mockHandleKeyboardEvent); + expect(mockHandleKeyboardEvent).toHaveBeenCalledWith(params); + }); + + it('flushes withheld keys upon non-withheld key press', () => { + const metaDown = { + e: { key: 'Meta' } as KeyboardEvent, + state: ButtonState.DOWN, + cli: new TdpClient('wss://socketAddr.gov'), + }; + + const metaUp = { + e: { key: 'Meta' } as KeyboardEvent, + state: ButtonState.UP, + cli: new TdpClient('wss://socketAddr.gov'), + }; + + const enterDown = { + e: { key: 'Enter' } as KeyboardEvent as KeyboardEvent, + state: ButtonState.DOWN, + cli: new TdpClient('wss://socketAddr.gov'), + }; + + withholder.handleKeyboardEvent(metaDown, mockHandleKeyboardEvent); + withholder.handleKeyboardEvent(metaUp, mockHandleKeyboardEvent); + + expect(mockHandleKeyboardEvent).not.toHaveBeenCalled(); + + withholder.handleKeyboardEvent(enterDown, mockHandleKeyboardEvent); + + expect(mockHandleKeyboardEvent).toHaveBeenCalledTimes(3); + expect(mockHandleKeyboardEvent).toHaveBeenNthCalledWith(1, metaDown); + expect(mockHandleKeyboardEvent).toHaveBeenNthCalledWith(2, metaUp); + expect(mockHandleKeyboardEvent).toHaveBeenNthCalledWith(3, enterDown); + }); + + it('withholds Meta/Alt UP and then handles them on a timer', () => { + const metaParams = { + e: { key: 'Meta' } as KeyboardEvent, + state: ButtonState.UP, + cli: new TdpClient('wss://socketAddr.gov'), + }; + const altParams = { + e: { key: 'Alt' } as KeyboardEvent, + state: ButtonState.UP, + cli: new TdpClient('wss://socketAddr.gov'), + }; + + withholder.handleKeyboardEvent(metaParams, mockHandleKeyboardEvent); + withholder.handleKeyboardEvent(altParams, mockHandleKeyboardEvent); + + expect(mockHandleKeyboardEvent).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(10); + + expect(mockHandleKeyboardEvent).toHaveBeenCalledTimes(2); + expect(mockHandleKeyboardEvent).toHaveBeenNthCalledWith(1, metaParams); + expect(mockHandleKeyboardEvent).toHaveBeenNthCalledWith(2, altParams); + }); + + it('cancels withheld keys correctly', () => { + const metaParams = { + e: { key: 'Meta' } as KeyboardEvent, + state: ButtonState.UP, + cli: new TdpClient('wss://socketAddr.gov'), + }; + withholder.handleKeyboardEvent(metaParams, mockHandleKeyboardEvent); + expect((withholder as any).withheldKeys).toHaveLength(1); + + withholder.cancel(); + jest.advanceTimersByTime(10); + + expect(mockHandleKeyboardEvent).not.toHaveBeenCalled(); + expect((withholder as any).withheldKeys).toHaveLength(0); + }); +}); diff --git a/web/packages/teleport/src/DesktopSession/Withholder.tsx b/web/packages/teleport/src/DesktopSession/Withholder.tsx new file mode 100644 index 0000000000000..00e2fdf528a18 --- /dev/null +++ b/web/packages/teleport/src/DesktopSession/Withholder.tsx @@ -0,0 +1,117 @@ +/** + * Teleport + * Copyright (C) 2023 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { ButtonState } from 'teleport/lib/tdp'; + +import { KeyboardEventParams } from './KeyboardHandler'; + +/** + * The Withholder class manages keyboard events, particularly for alt/cmd keys. It delays handling these keys to determine the user's intent: + * + * - For alt/cmd DOWN events, it waits to see if this is part of a normal operation or an alt/cmd + tab action. In the latter case, it cancels + * the event to prevent the remote server from mistakenly thinking the key is held down when the user returns to the browser window, which can + * cause issues as described in https://github.com/gravitational/teleport/issues/24342. + * - For alt/cmd UP events, it introduces a short delay before handling to avoid unintended actions (like opening the start menu), in the case + * that an alt/cmd + tab registers both the alt/cmd DOWN and UP events in quick succession. (This can happen if the user does an alt/cmd + tab + * really quickly, according to our testing.) + * - For other keys, it handles them immediately. + * + * Events are either processed immediately, delayed, or cancelled based on user actions and focus changes. + */ +export class Withholder { + /** + * The list of keys which are to be withheld. + */ + private keysToWithhold: string[] = ['Meta', 'Alt']; + /** + * The internal array of keystrokes that are currently + * being withheld. + */ + private withheldKeys: Array = []; + + /** + * All keyboard events should be handled via this function. + */ + public handleKeyboardEvent( + params: KeyboardEventParams, + handleKeyboardEvent: (params: KeyboardEventParams) => void + ) { + const key = params.e.key; + + // If this is not a key we withhold, immediately flush any withheld keys + // and handle this key. + if (!this.keysToWithhold.includes(key)) { + this.flush(); + handleKeyboardEvent(params); + return; + } + + // This is a key we withhold: + + // On key down we withhold without a timeout. The handler will ultimately be called + // when the key is released (typically after a timeout, see the comment in the + // conditional below), or when another key is pressed, or else it should be cancelled + // onfocusout or on unmount. + let timeout = undefined; + if (params.state === ButtonState.UP) { + // On key ups we withhold on a timeout. The function will be called when the + // timer times out, or when another key is pressed, or else it should be + // cancelled onfocusout or on unmount. + timeout = setTimeout(() => { + // Just flush after the timeout, the handler will + // be in the queue by then and thus will be called. + // + // Technically this might flush some keys that were + // pressed after this one, but that works okay in practice. + // A user would have to be doing something extremely unusual + // for this to become a noticeable problem. + this.flush(); + }, UP_DELAY_MS); + } + + this.withheldKeys.push({ + params, + handler: handleKeyboardEvent, + timeout, + }); + } + + // Cancel all withheld keys. + public cancel() { + this.withheldKeys.forEach(w => clearTimeout(w.timeout)); + this.withheldKeys = []; + } + + // Flush all withheld keys. + private flush() { + this.withheldKeys.forEach(w => { + clearTimeout(w.timeout); + w.handler(w.params); + }); + this.withheldKeys = []; + } +} + +type WithheldKeyboardEventHandler = { + handler: (params: KeyboardEventParams) => void; + params: KeyboardEventParams; + timeout?: NodeJS.Timeout; +}; + +// 10 ms was determined empirically to work well. +const UP_DELAY_MS = 10; diff --git a/web/packages/teleport/src/DesktopSession/__snapshots__/DesktopSession.story.test.tsx.snap b/web/packages/teleport/src/DesktopSession/__snapshots__/DesktopSession.story.test.tsx.snap index 60c2fdc197353..d94ecf8bd5b38 100644 --- a/web/packages/teleport/src/DesktopSession/__snapshots__/DesktopSession.story.test.tsx.snap +++ b/web/packages/teleport/src/DesktopSession/__snapshots__/DesktopSession.story.test.tsx.snap @@ -444,7 +444,7 @@ exports[`connected settings false 1`] = ` > - some connection error + some tdp error Refresh the page to reconnect. @@ -1133,12 +1133,6 @@ exports[`disconnected 1`] = ` color: rgba(255,255,255,0.36); } -.c16 { - box-sizing: border-box; - margin: 72px; - text-align: center; -} - .c14 { align-items: center; border: none; @@ -1187,12 +1181,6 @@ exports[`disconnected 1`] = ` padding-right: 16px; } -.c17 { - overflow: hidden; - text-overflow: ellipsis; - margin: 0px; -} - .c8 { display: inline-flex; align-items: center; @@ -1392,15 +1380,6 @@ exports[`disconnected 1`] = ` -
-
- Session successfully disconnected -
-
`; -exports[`invalid processing 1`] = ` -.c11 { +exports[`fetch processing 1`] = ` +.c9 { line-height: 1.5; margin: 0; display: inline-flex; @@ -1632,185 +1611,312 @@ exports[`invalid processing 1`] = ` text-transform: uppercase; transition: all 0.3s; -webkit-font-smoothing: antialiased; - color: #FFFFFF; - background: rgba(255,255,255,0.07); - min-height: 40px; + color: #000000; + background: #9F85FF; + min-height: 32px; font-size: 12px; - padding: 0px 40px; - width: 30%; + padding: 0px 24px; + padding-left: 8px; + padding-right: 8px; } -.c11:hover, -.c11:focus { - background: rgba(255,255,255,0.13); +.c9:hover, +.c9:focus { + background: #B29DFF; } -.c11:active { - background: rgba(255,255,255,0.18); +.c9:active { + background: #C5B6FF; } -.c11:disabled { +.c9:disabled { background: rgba(255,255,255,0.12); color: rgba(255,255,255,0.3); cursor: auto; } -.c9 { - display: flex; - align-items: center; - justify-content: center; - border-radius: 2px; +.c0 { box-sizing: border-box; - box-shadow: 0 1px 4px rgba(0,0,0,0.24); - margin: 0 0 24px 0; - min-height: 40px; - padding: 8px 16px; - overflow: auto; - word-break: break-word; - line-height: 1.5; - background: #039be5; - color: #000000; } -.c9 a { - color: #FFFFFF; +.c2 { + box-sizing: border-box; + height: 40px; + background-color: #000000; + flex: 0 0 auto; } -.c10 { +.c5 { box-sizing: border-box; + padding-left: 16px; + padding-right: 16px; } -.c4 { +.c11 { box-sizing: border-box; - margin-bottom: 16px; - min-height: 32px; + color: rgba(255,255,255,0.36); } -.c7 { +.c16 { box-sizing: border-box; - margin-bottom: 32px; - flex: 1; + margin: 72px; + text-align: center; } -.c6 { +.c14 { + align-items: center; + border: none; + cursor: pointer; + display: flex; + outline: none; + border-radius: 50%; + overflow: visible; + justify-content: center; + text-align: center; + flex: 0 0 auto; + background: transparent; + color: inherit; + transition: all 0.3s; + -webkit-font-smoothing: antialiased; + font-size: 12px; + height: 24px; + width: 24px; + margin-left: 24px; + color: rgba(255,255,255,0.72); +} + +.c14:disabled { + color: rgba(255,255,255,0.36); +} + +.c14:disabled { + color: rgba(255,255,255,0.36); + cursor: default; +} + +.c14:hover:enabled, +.c14:focus:enabled { + background: rgba(255,255,255,0.13); +} + +.c14:active:enabled { + background: rgba(255,255,255,0.18); +} + +.c4 { overflow: hidden; text-overflow: ellipsis; - font-weight: 300; - font-size: 22px; - line-height: 32px; - text-transform: uppercase; margin: 0px; - color: #FFFFFF; + padding-left: 16px; + padding-right: 16px; } .c8 { + display: inline-flex; + align-items: center; + justify-content: center; + padding-right: 16px; +} + +.c13 { + display: inline-flex; + align-items: center; + justify-content: center; + margin-right: 8px; +} + +.c15 { + display: inline-flex; + align-items: center; + justify-content: center; +} + +.c1 { display: flex; flex-direction: column; } -.c5 { +.c3 { display: flex; align-items: center; + flex-direction: row; } -.c1 { - z-index: -1; - position: fixed; - right: 0; - bottom: 0; - top: 0; - left: 0; - background-color: rgba(0,0,0,0.5); - opacity: 1; - touch-action: none; +.c6 { + display: flex; } -.c0 { - position: fixed; - z-index: 1200; - right: 0; - bottom: 0; - top: 0; - left: 0; +.c7 { + display: flex; + align-items: center; } -.c2 { - height: 100%; - outline: none; - color: black; +.c12 { display: flex; align-items: center; - justify-content: center; - opacity: 1; - will-change: opacity; - transition: opacity 225ms cubic-bezier(0.4,0,0.2,1) 0ms; + justify-content: space-between; } -.c3 { - padding: 32px; - padding-top: 24px; - background: #222C59; +.c10 { color: #FFFFFF; - border-radius: 8px; - box-shadow: 0 8px 32px rgba(0,0,0,0.24); - display: flex; - flex-direction: column; - position: relative; - overflow-y: auto; - max-height: calc(100% - 96px); - width: 484px; + min-height: 0; + height: 24px; + background-color: rgba(255,255,255,0.13); } -
-
- Session disconnected for an unknown reason. + websocket disconnected for an unknown reason
Refresh the page to reconnect.
diff --git a/web/packages/teleport/src/DesktopSession/useDesktopSession.tsx b/web/packages/teleport/src/DesktopSession/useDesktopSession.tsx index ccd2edcd27801..78ae050f01b8a 100644 --- a/web/packages/teleport/src/DesktopSession/useDesktopSession.tsx +++ b/web/packages/teleport/src/DesktopSession/useDesktopSession.tsx @@ -36,17 +36,19 @@ export default function useDesktopSession() { // tdpConnection tracks the state of the tdpClient's TDP connection // - 'processing' at first // - 'success' once the first TdpClientEvent.IMAGE_FRAGMENT is seen - // - 'failed' if a fatal error is encountered + // - 'failed' if a fatal error is encountered, should have a statusText + // - '' if the connection closed gracefully by the server, should have a statusText const { attempt: tdpConnection, setAttempt: setTdpConnection } = useAttempt('processing'); // wsConnection track's the state of the tdpClient's websocket connection. - // 'closed' to start, 'open' when TdpClientEvent.WS_OPEN is encountered, then 'closed' - // again when TdpClientEvent.WS_CLOSE is encountered. - const [wsConnection, setWsConnection] = useState<'open' | 'closed'>('closed'); - - // disconnected tracks whether the user intentionally disconnected the client - const [disconnected, setDisconnected] = useState(false); + // - 'init' to start + // - 'open' when TdpClientEvent.WS_OPEN is encountered + // - then 'closed' again when TdpClientEvent.WS_CLOSE is encountered. + // Once it's 'closed', it should have the message that came with the TdpClientEvent.WS_CLOSE event.. + const [wsConnection, setWsConnection] = useState({ + status: 'init', + }); const { username, desktopName, clusterId } = useParams(); @@ -193,8 +195,6 @@ export default function useDesktopSession() { fetchAttempt, tdpConnection, wsConnection, - disconnected, - setDisconnected, webauthn, setTdpConnection, showAnotherSessionActiveDialog, @@ -350,3 +350,8 @@ export const defaultDirectorySharingState: DirectorySharingState = { export const defaultClipboardSharingState: ClipboardSharingState = { browserSupported: navigator.userAgent.includes('Chrome'), }; + +export type WebsocketAttempt = { + status: 'init' | 'open' | 'closed'; + statusText?: string; +}; diff --git a/web/packages/teleport/src/DesktopSession/useTdpClientCanvas.tsx b/web/packages/teleport/src/DesktopSession/useTdpClientCanvas.tsx index e5f3c986a3f21..22bfae686050b 100644 --- a/web/packages/teleport/src/DesktopSession/useTdpClientCanvas.tsx +++ b/web/packages/teleport/src/DesktopSession/useTdpClientCanvas.tsx @@ -21,14 +21,11 @@ import { useState, useEffect, useRef } from 'react'; import { Attempt } from 'shared/hooks/useAttemptNext'; import { NotificationItem } from 'shared/components/Notification'; -import { Platform, getPlatform } from 'design/platform'; - import { TdpClient, ButtonState, ScrollAxis } from 'teleport/lib/tdp'; import { ClientScreenSpec, ClipboardData, PngFrame, - SyncKeys, } from 'teleport/lib/tdp/codec'; import { getHostName } from 'teleport/services/api'; import cfg from 'teleport/config'; @@ -44,6 +41,7 @@ import { defaultDirectorySharingState, isSharingClipboard, } from './useDesktopSession'; +import { KeyboardHandler } from './KeyboardHandler'; import type { BitmapFrame } from 'teleport/lib/tdp/client'; @@ -69,16 +67,16 @@ export default function useTdpClientCanvas(props: Props) { const initialTdpConnectionSucceeded = useRef(false); const encoder = useRef(new TextEncoder()); const latestClipboardDigest = useRef(''); + const keyboardHandler = useRef(new KeyboardHandler()); - /** - * Tracks whether the next keydown or keyup event should sync the - * local toggle key state to the remote machine. - * - * Set to true: - * - On component initialization, so keys are synced before the first keydown/keyup event. - * - On focusout, so keys are synced when the user returns to the window. - */ - const syncBeforeNextKey = useRef(true); + useEffect(() => { + keyboardHandler.current = new KeyboardHandler(); + // On unmount, clear all the timeouts on the keyboardHandler. + return () => { + // eslint-disable-next-line react-hooks/exhaustive-deps + keyboardHandler.current.dispose(); + }; + }, []); useEffect(() => { const addr = cfg.api.desktopWsAddr @@ -190,102 +188,35 @@ export default function useTdpClientCanvas(props: Props) { }); }; - const clientOnWsClose = () => { - setWsConnection('closed'); - }; - - const clientOnWsOpen = () => { - setWsConnection('open'); - }; - - /** - * Returns the ButtonState corresponding to the given `keyArg`. - * - * @param e The `KeyboardEvent` - * @param keyArg The key to check the state of. Valid values can be found [here](https://www.w3.org/TR/uievents-key/#keys-modifier) - */ - const getModifierState = (e: KeyboardEvent, keyArg: string): ButtonState => { - return e.getModifierState(keyArg) ? ButtonState.DOWN : ButtonState.UP; - }; - - const getSyncKeys = (e: KeyboardEvent): SyncKeys => { - return { - scrollLockState: getModifierState(e, 'ScrollLock'), - numLockState: getModifierState(e, 'NumLock'), - capsLockState: getModifierState(e, 'CapsLock'), - kanaLockState: ButtonState.UP, // KanaLock is not supported, see https://www.w3.org/TR/uievents-key/#keys-modifier - }; - }; - - /** - * Called before every keydown or keyup event. - * - * If syncBeforeNextKey is true, this function - * synchronizes the keys to the remote machine. - */ - const handleSyncBeforeNextKey = (cli: TdpClient, e: KeyboardEvent) => { - if (syncBeforeNextKey.current === true) { - cli.sendSyncKeys(getSyncKeys(e)); - syncBeforeNextKey.current = false; - } + const clientOnTdpInfo = (info: string) => { + setDirectorySharingState(defaultDirectorySharingState); + setClipboardSharingState(defaultClipboardSharingState); + setTdpConnection({ + status: '', // gracefully disconnecting + statusText: info, + }); }; - const isMac = getPlatform() === Platform.macOS; - /** - * Special handler for the CapsLock key. - * - * On MacOS Edge/Chrome/Safari, each physical CapsLock DOWN-UP registers - * as either a single DOWN or single UP, with DOWN corresponding to - * "CapsLock on" and UP to "CapsLock off". On MacOS Firefox, it always - * registers as a DOWN. - * - * On Windows and Linux, all browsers treat CapsLock like a normal key. - * - * The remote Windows machine also treats CapsLock like a normal key, and - * expects a DOWN-UP whenever it's pressed. - */ - const handleCapsLock = (cli: TdpClient, state: ButtonState) => { - if (isMac) { - // On Mac, every UP or DOWN given to us by the browser corresponds - // to a DOWN + UP on the remote machine. - cli.sendKeyboardInput('CapsLock', ButtonState.DOWN); - cli.sendKeyboardInput('CapsLock', ButtonState.UP); - } else { - // On Windows or Linux, we just pass the event through normally to the server. - cli.sendKeyboardInput('CapsLock', state); - } + const clientOnWsClose = (statusText: string) => { + setWsConnection({ status: 'closed', statusText }); }; - /** - * Handles a keyboard event. - */ - const handleKeyboardEvent = ( - cli: TdpClient, - e: KeyboardEvent, - state: ButtonState - ) => { - if (e.code === 'CapsLock') { - handleCapsLock(cli, state); - return; - } - cli.sendKeyboardInput(e.code, state); + const clientOnWsOpen = () => { + setWsConnection({ status: 'open' }); }; const canvasOnKeyDown = (cli: TdpClient, e: KeyboardEvent) => { - e.preventDefault(); - handleSyncBeforeNextKey(cli, e); - handleKeyboardEvent(cli, e, ButtonState.DOWN); + keyboardHandler.current.handleKeyboardEvent({ + cli, + e, + state: ButtonState.DOWN, + }); // The key codes in the if clause below are those that have been empirically determined not // to count as transient activation events. According to the documentation, a keydown for // the Esc key and any "shortcut key reserved by the user agent" don't count as activation // events: https://developer.mozilla.org/en-US/docs/Web/Security/User_activation. - if ( - e.code !== 'MetaRight' && - e.code !== 'MetaLeft' && - e.code !== 'AltRight' && - e.code !== 'AltLeft' - ) { + if (e.key !== 'Meta' && e.key !== 'Alt' && e.key !== 'Escape') { // Opportunistically sync local clipboard to remote while // transient user activation is in effect. // https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/readText#security @@ -294,13 +225,15 @@ export default function useTdpClientCanvas(props: Props) { }; const canvasOnKeyUp = (cli: TdpClient, e: KeyboardEvent) => { - e.preventDefault(); - handleSyncBeforeNextKey(cli, e); - handleKeyboardEvent(cli, e, ButtonState.UP); + keyboardHandler.current.handleKeyboardEvent({ + cli, + e, + state: ButtonState.UP, + }); }; const canvasOnFocusOut = () => { - syncBeforeNextKey.current = true; + keyboardHandler.current.onFocusOut(); }; const canvasOnMouseMove = ( @@ -375,6 +308,7 @@ export default function useTdpClientCanvas(props: Props) { clientOnWsClose, clientOnWsOpen, clientOnTdpWarning, + clientOnTdpInfo, canvasOnKeyDown, canvasOnKeyUp, canvasOnFocusOut, @@ -401,7 +335,7 @@ type Props = { desktopName: string; clusterId: string; setTdpConnection: Setter; - setWsConnection: Setter<'open' | 'closed'>; + setWsConnection: Setter<{ status: 'open' | 'closed'; statusText?: string }>; clipboardSharingState: ClipboardSharingState; setClipboardSharingState: Setter; setDirectorySharingState: Setter; diff --git a/web/packages/teleport/src/Player/DesktopPlayer.tsx b/web/packages/teleport/src/Player/DesktopPlayer.tsx index 8262835db060c..c7602e688d402 100644 --- a/web/packages/teleport/src/Player/DesktopPlayer.tsx +++ b/web/packages/teleport/src/Player/DesktopPlayer.tsx @@ -63,6 +63,7 @@ export const DesktopPlayer = ({ clientOnClientScreenSpec, clientOnWsClose, clientOnTdpError, + clientOnTdpInfo, } = useDesktopPlayer({ sid, clusterId, @@ -102,6 +103,7 @@ export const DesktopPlayer = ({ clientOnClientScreenSpec={clientOnClientScreenSpec} clientOnWsClose={clientOnWsClose} clientOnTdpError={clientOnTdpError} + clientOnTdpInfo={clientOnTdpInfo} canvasOnContextMenu={handleContextMenu} style={{ ...canvasStyle, @@ -159,7 +161,7 @@ const useDesktopPlayer = ({ clusterId, sid }) => { .replace(':clusterId', clusterId) .replace(':sid', sid); return new PlayerClient({ url, setTime, setPlayerStatus, setStatusText }); - }, [clusterId, sid, setTime, setPlayerStatus]); + }, [clusterId, sid]); const clientOnWsClose = useCallback(() => { if (playerClient) { @@ -167,13 +169,15 @@ const useDesktopPlayer = ({ clusterId, sid }) => { } }, [playerClient]); - const clientOnTdpError = useCallback( - (error: Error) => { - setPlayerStatus(StatusEnum.ERROR); - setStatusText(error.message || error.toString()); - }, - [setPlayerStatus, setStatusText] - ); + const clientOnTdpError = useCallback((error: Error) => { + setPlayerStatus(StatusEnum.ERROR); + setStatusText(error.message || error.toString()); + }, []); + + const clientOnTdpInfo = useCallback((info: string) => { + setPlayerStatus(StatusEnum.COMPLETE); + setStatusText(info); + }, []); const clientOnClientScreenSpec = useCallback( (_cli: TdpClient, canvas: HTMLCanvasElement, spec: ClientScreenSpec) => { @@ -200,7 +204,7 @@ const useDesktopPlayer = ({ clusterId, sid }) => { setCanvasSizeIsSet(true); }, - [setCanvasSizeIsSet] + [] ); useEffect(() => { @@ -221,6 +225,7 @@ const useDesktopPlayer = ({ clusterId, sid }) => { clientOnClientScreenSpec, clientOnWsClose, clientOnTdpError, + clientOnTdpInfo, }; }; diff --git a/web/packages/teleport/src/components/TdpClientCanvas/TdpClientCanvas.tsx b/web/packages/teleport/src/components/TdpClientCanvas/TdpClientCanvas.tsx index 2963c79d05975..a03c1e05bc502 100644 --- a/web/packages/teleport/src/components/TdpClientCanvas/TdpClientCanvas.tsx +++ b/web/packages/teleport/src/components/TdpClientCanvas/TdpClientCanvas.tsx @@ -38,6 +38,7 @@ function TdpClientCanvas(props: Props) { clientOnClipboardData, clientOnTdpError, clientOnTdpWarning, + clientOnTdpInfo, clientOnWsClose, clientOnWsOpen, clientOnClientScreenSpec, @@ -226,6 +227,16 @@ function TdpClientCanvas(props: Props) { } }, [client, clientOnTdpWarning]); + useEffect(() => { + if (client && clientOnTdpInfo) { + client.on(TdpClientEvent.TDP_INFO, clientOnTdpInfo); + + return () => { + client.removeListener(TdpClientEvent.TDP_INFO, clientOnTdpInfo); + }; + } + }, [client, clientOnTdpInfo]); + useEffect(() => { if (client && clientOnWsClose) { client.on(TdpClientEvent.WS_CLOSE, clientOnWsClose); @@ -408,7 +419,8 @@ export type Props = { clientOnClipboardData?: (clipboardData: ClipboardData) => void; clientOnTdpError?: (error: Error) => void; clientOnTdpWarning?: (warning: string) => void; - clientOnWsClose?: () => void; + clientOnTdpInfo?: (info: string) => void; + clientOnWsClose?: (message: string) => void; clientOnWsOpen?: () => void; clientOnClientScreenSpec?: ( cli: TdpClient, diff --git a/web/packages/teleport/src/ironrdp/src/lib.rs b/web/packages/teleport/src/ironrdp/src/lib.rs index f448392f59a8e..bb4c8a517d1f6 100644 --- a/web/packages/teleport/src/ironrdp/src/lib.rs +++ b/web/packages/teleport/src/ironrdp/src/lib.rs @@ -248,7 +248,7 @@ impl FastPathProcessor { let frame = Uint8Array::from(frame.as_slice()); // todo(isaiah): this is a copy let _ = respond_cb.call1(cb_context, &frame.buffer())?; } - ActiveStageOutput::Terminate => { + ActiveStageOutput::Terminate(_) => { return Err(JsValue::from_str("Terminate should never be returned")); } ActiveStageOutput::PointerBitmap(pointer) => { diff --git a/web/packages/teleport/src/lib/tdp/client.ts b/web/packages/teleport/src/lib/tdp/client.ts index 78815dfb012a4..8335bdcd8dbf1 100644 --- a/web/packages/teleport/src/lib/tdp/client.ts +++ b/web/packages/teleport/src/lib/tdp/client.ts @@ -71,6 +71,8 @@ export enum TdpClientEvent { TDP_WARNING = 'tdp warning', // CLIENT_WARNING represents a warning event that isn't a TDP_WARNING CLIENT_WARNING = 'client warning', + // TDP_INFO corresponds with the TDP info message + TDP_INFO = 'tdp info', WS_OPEN = 'ws open', WS_CLOSE = 'ws close', RESET = 'reset', @@ -135,7 +137,12 @@ export default class Client extends EventEmitterWebAuthnSender { // prior to a socket 'close' event (https://stackoverflow.com/a/40084550/6277051). // Therefore, we can rely on our onclose handler to account for any websocket errors. this.socket.onerror = null; - this.socket.onclose = () => { + this.socket.onclose = ev => { + let message = 'session disconnected'; + if (ev.code !== WebsocketCloseCode.NORMAL) { + this.logger.error(`websocket closed with error code: ${ev.code}`); + message = `connection closed with websocket error`; + } this.logger.info('websocket is closed'); // Clean up all of our socket's listeners and the socket itself. @@ -144,7 +151,7 @@ export default class Client extends EventEmitterWebAuthnSender { this.socket.onclose = null; this.socket = null; - this.emit(TdpClientEvent.WS_CLOSE); + this.emit(TdpClientEvent.WS_CLOSE, message); }; } @@ -295,6 +302,8 @@ export default class Client extends EventEmitterWebAuthnSender { ); } else if (notification.severity === Severity.Warning) { this.handleWarning(notification.message, TdpClientEvent.TDP_WARNING); + } else { + this.handleInfo(notification.message); } } @@ -704,6 +713,11 @@ export default class Client extends EventEmitterWebAuthnSender { this.emit(warnType, warning); } + private handleInfo(info: string) { + this.logger.info(info); + this.emit(TdpClientEvent.TDP_INFO, info); + } + // Ensures full cleanup of this object. // Note that it removes all listeners first and then cleans up the socket, // so don't call this if your calling object is relying on listeners. diff --git a/web/packages/teleport/src/lib/useWebAuthn.ts b/web/packages/teleport/src/lib/useWebAuthn.ts index 50eddd9ce5846..730065299ceed 100644 --- a/web/packages/teleport/src/lib/useWebAuthn.ts +++ b/web/packages/teleport/src/lib/useWebAuthn.ts @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -import { useState, useEffect } from 'react'; +import { useState, useEffect, Dispatch, SetStateAction } from 'react'; import { EventEmitterWebAuthnSender } from 'teleport/lib/EventEmitterWebAuthnSender'; import { TermEvent } from 'teleport/lib/term/enums'; @@ -25,7 +25,9 @@ import { makeWebauthnAssertionResponse, } from 'teleport/services/auth'; -export default function useWebAuthn(emitterSender: EventEmitterWebAuthnSender) { +export default function useWebAuthn( + emitterSender: EventEmitterWebAuthnSender +): WebAuthnState { const [state, setState] = useState({ addMfaToScpUrls: false, requested: false, @@ -96,3 +98,18 @@ export default function useWebAuthn(emitterSender: EventEmitterWebAuthnSender) { addMfaToScpUrls: state.addMfaToScpUrls, }; } + +export type WebAuthnState = { + errorText: string; + requested: boolean; + authenticate: () => void; + setState: Dispatch< + SetStateAction<{ + addMfaToScpUrls: boolean; + requested: boolean; + errorText: string; + publicKey: PublicKeyCredentialRequestOptions; + }> + >; + addMfaToScpUrls: boolean; +};