Support directory sharing in Connect #54663
Merged
Conversation
gzdunek
added
no-changelog
gzdunek
force-pushed
the
gzdunek/fs-operations-tshd
branch
from
51f16ea to
ec82342
Compare
gzdunek
force-pushed
the
gzdunek/dir-sharing-connect
branch
from
76ff1b3 to
126a2f1
Compare
ravicious
reviewed
May 12, 2025
ravicious
approved these changes
May 13, 2025
| ReadDataLength: uint32(len(contents)), | ||
| ReadData: contents, | ||
| ReadDataLength: uint32(n), | ||
| ReadData: buf[:n], |
Member
There was a problem hiding this comment.
Is there an equivalent of this code in the Web UI so that I can compare implementation? I guess there isn't since in the Web UI it's done by the browser API, right?
It's just that I don't have good heuristics for reviewing code that works on buffers, so I worry that I might miss something. ;f
Contributor
Author
There was a problem hiding this comment.
The code for Web UI is here, but it uses File System API:
zmb3
reviewed
May 13, 2025
…renderer process
gzdunek
force-pushed
the
gzdunek/dir-sharing-connect
branch
from
ec22129 to
bd8f84c
Compare
Contributor
Author
|
Friendly ping @zmb3 |
zmb3
approved these changes
May 15, 2025
# Conflicts: # gen/proto/go/teleport/lib/teleterm/v1/service.pb.go # gen/proto/go/teleport/lib/teleterm/v1/service_grpc.pb.go
Contributor
gzdunek
added a commit
that referenced
this pull request
May 19, 2025
* Register directory access when starting a desktop session * Add RPC to attach a directory to desktop session * Do not allow `attachDirectoryToDesktopSession` to be called from the renderer process * Open the directory picker and send the selected path to tshd * Intercept file system events coming from the server and handle them * Disallow file system messages to be sent from the renderer * Refactor dir sharing * `AttachDirectoryToDesktopSession` -> `SetSharedDirectoryForDesktopSession` * Improve comments * Small fixes * Add missing defer for `s.dirAccessMu.RUnlock()` * `TestOpenSharedDirectory` -> `TestNewDirectoryAccess` * Add a comment for JS file system handlers * `make grpc` --------- Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> (cherry picked from commit 45428c0)
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Jun 2, 2025
…54926) * Show Windows desktops in Connect (#53955) * Show Windows desktops in Connect * Use `net.SplitHostPort` * Use underscore in URIs * Omit the default RDP port before displaying item in the UI (cherry picked from commit 81af813) * Add desktop session-related ACLs to Connect (#54031) * Add desktop session-related ACLs to Connect * Add new fields to `makeAcl` (cherry picked from commit aba0de0) * Add latency detector for desktop sessions (#52827) * wip * add ping message and latency from desktop side * version * Fix backward compatibility * godocs * formatting * fix imports * formatting * formatting * log and gci * lint * Apply tooltip on the icon directly, instead of on the Menu component This fixes a problem where onMouseLeave in HoverTooltip wasn't called and the tooltip didn't disappear. * Use consistent spacing between top bar elements * updates from origin * e * e * lint * rework UI after merge * Rename fields in backend * prettier * Update web/packages/shared/components/DesktopSession/TopBar.tsx Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com> * review comments * fix ui * add env var to disable windows desktop "ping" * review comment * review comment * Update lib/web/desktop.go Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com> * Refactor latency monitoring * fix spelling * remove monitorSessionLatency * gci * review comment * review comment * Update RFD and version * fix gaps --------- Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com> Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com> Co-authored-by: Grzegorz Zdunek <gzdunek@users.noreply.github.com> (cherry picked from commit 1aba870) * Abstract directory sharing file system (#54545) * Extract an interface to interact with a shared directory file system Some methods were renamed to more closely follow common file system naming conventions. * Use the new interface in the TDP client * Provide a browser file system as the shared directory access implementation (cherry picked from commit d3fbeeb) * Support desktop access in Connect (#54373) * Extract reusable function for establishing connections to Windows Desktop Service * Add `ProxyWindowsDesktopSession` proto * Implement `ProxyWindowsDesktopSession` * Enable fetching desktops and desktop services in remote proxy cache * Implement dialing windows desktop * Implement client * Support Windows desktop certs in tsh * Fix incorrect `windowsDesktop` URI * Add proto for `ConnectToDesktop` * Implement `ConnectToDesktop` * Do not log requests/responses for `ConnectToDesktop` RPC * Add boilerplate for `DocumentDesktopSession` * Open a desktop connection * Relax ArrayBuffer type passed to encode methods, ignore tshd abort errors In tshd stream, the buffer is of type `ArrayBufferLike` (which is `ArrayBuffer` & `SharedArrayBuffer`). To allow assigning it to the type in our TDP code, we make it more general. * Ensure WASM IronRDP code is initialized only once * Use `utils.ShuffleVisit` * Improve stream cancellation handling * Leave a TODO about ListWindowsDesktops * Do not return empty data slice * Provide non-nil src and dest addresses to `streamutils.NewConn()` * Do not emit an empty message to indicate a successful connection * Fix test * Simplify code * Add missing `WindowsDesktopTLSCredentials` initialization * Require that the first message is only a dial request and the subsequent ones are only data * Add explicit `stop()` check * Hold cluster name and desktop name in a struct for the map key * Do not return early on non-connection problem errors * Handle io.EOF error specifically in BidiStreamingClient.Send instead of in `tlsConn.HandshakeContext` * Extract a common function to proxy TDP connections * Improve proto comments and connection setup * Add comments and logs * Lint * Explain why there's a special handling for abort error * Bring back the original `proxyWebsocketConn` behavior when it comes to error handling * Post merge fixes * Adjust proxying TDP connection to changes from master * Lint * Channels improvements * Post merge fixes (cherry picked from commit 980ce61) * Show desktops in connection tracker (#54668) * Make supporting non-Windows desktops easier in the future * Add connection boilerplate for desktop connections * Show connected/disconnected status in the connection tracker * Make sure that ACLs were fetched before reading from them ACLs are fetched asynchronously. * Do not crash when reading unsupported connection from app_state.json * `gwDoc` -> `doc` * `windowsDesktops` -> `windows_desktops` (cherry picked from commit 9f49376) * Implement shared directory file system in tsh daemon (#54662) * Implement shared directory file system in tshd * Remove `Open` method * Do not use `os.IsNotExist` * Read bytes into `[]byte` parameter * Correctly use `t.Helper()` * Add missing `trace.Wrap` * Rename receivers * Handle errors from `file.Close()` * Use named returns to return close error * Improve error handling (cherry picked from commit b3e9199) * Support directory sharing in Connect (#54663) * Register directory access when starting a desktop session * Add RPC to attach a directory to desktop session * Do not allow `attachDirectoryToDesktopSession` to be called from the renderer process * Open the directory picker and send the selected path to tshd * Intercept file system events coming from the server and handle them * Disallow file system messages to be sent from the renderer * Refactor dir sharing * `AttachDirectoryToDesktopSession` -> `SetSharedDirectoryForDesktopSession` * Improve comments * Small fixes * Add missing defer for `s.dirAccessMu.RUnlock()` * `TestOpenSharedDirectory` -> `TestNewDirectoryAccess` * Add a comment for JS file system handlers * `make grpc` --------- Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> (cherry picked from commit 45428c0) * Simplify code around latency detection for desktop (#54795) * Simplify code around latency detection for desktop * cleanup * Use tdp.Conn for client and server * Add check for context cancellation * Cleanup * remove context check * remove context check * remove unused field * use sync.OnceFunc, error on unexpected ping * err message (cherry picked from commit 28f78c3) * Show `desktop access requires Teleport Proxy 17.5.0 or higher` when Proxy returns NotImplementedError * Add `wasm-unsafe-eval` to Connect CSP (#54916) (cherry picked from commit ee8be4d) * Ensure all errors thrown by `TdpClient` are instances of `Error` class (#54877) * Add a utility function to convert any input to an Error instance * Add types for TdpClient events * Always throw Error from `adaptWebSocketToTdpTransport` * Improve getting the error message in `getErrMessage` * Move `isAbortError` to error.ts * Remove unused `tshd/errors.ts` * Handle `JSON.stringify` error, add `cause` to thrown errors * Remove unnecessary `async` * Remove `error.toString` from the callsites * Handle `err` being undefined (cherry picked from commit 4d28408) --------- Co-authored-by: Przemko Robakowski <przemko.robakowski@goteleport.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributes to #20802
Part 2/2 of directory sharing in Connect.
This works by intercepting TDP file system messages and handling them in tshd.
The process begins when the Electron main process opens a directory picker.
Once a path is selected, it is passed to tshd via the
AttachDirectoryToDesktopSessionAPI (for security it's not allowed to call this from the renderer process).tsh daemon then verifies whether there is an active session for the specified desktop user and attempts to open the directory.
Once that's done, everything is ready on the tsh daemon to intercept and handle the file system events.
The final step is sending a
SharedDirectoryAnnouncemessage to the server, which is done from the JavaScript renderer process. This message is safe to send from the renderer because it only provides a display name for the mounted drive on the remote machine. It has no effect on local file system operations.Best to review commit by commit. The diff count seems large, but over 400 lines come from the generated proto.