diff --git a/README.md b/README.md index 6134837..da309bd 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,7 @@ Three consequences worth knowing before you read further: The reasoning behind each major decision is recorded in [`docs/adr/`](docs/adr/), starting with [the E2EE trust model](docs/adr/0001-e2ee-trust-model.md). -The desktop client's interface was built from a design covering more product than exists yet — file +The desktop client's interface was built from a design covering more product than existed at the time — file transfer, teams, saved snippets, port forwarding. Everything that design asked for and this build has not got is written down in [`docs/design-import-gaps.md`](docs/design-import-gaps.md), with the layer each piece would land in and what the interface shows in its place. Nothing was rendered with invented data to @@ -426,6 +426,32 @@ What is not here: transferring a directory, dragging between the panes, and rout bastion — the last needs jump hosts the connection layer has not got. All three are in [`docs/design-import-gaps.md`](docs/design-import-gaps.md). +### Forwarding ports + +A host can carry **port forwards**, added under PORT FORWARDS in its editor, in the three kinds OpenSSH has: +**local** (`-L`, a port here reaching a machine the remote can see), **remote** (`-R`, a port on the remote +reaching back to one this machine can see) and **dynamic** (`-D`, a SOCKS proxy here whose connections leave +from the remote). They are saved in the host's encrypted payload, so they sync, merge and are shared with the +host, and they start with every terminal opened to it, on either head. The session sidebar lists them under +FORWARDS with the state of each, and the status bar counts the ones listening; on the phone the same list is a +sheet off the terminal's bar. + +Three things about them are worth expecting rather than discovering. + +**Everything listens on localhost, and there is no setting for it.** A forward is a field any member of a +shared keychain can fill in, and one bound to every interface would hand the remote's network to anybody who +can reach your machine for as long as the terminal is open. A remote forward asks the remote for its loopback +too; whether the remote honours that is its own `GatewayPorts`. + +**A forward lives as long as its terminal.** It runs over that tab's connection, so closing the tab, losing +the connection or reconnecting stops it — a reconnect starts the host's forwards again on the new session. A +second tab to the same host runs the same forwards, and they fail there because the ports are taken; the row +says so, and the terminal is unaffected. Any row can be switched off and on while its session is up. + +**No byte counts**, which the design draws: SSH.NET keeps none, so a number there would be invented. The +phone runs forwards too, listed behind a `⇄` in its terminal's bar, but cannot edit them yet; see +[The Android head](#the-android-head). + ### Reaching a server through Spelunk A server managed by Spelunk runs Spelunk's agent, which keeps a connection open to Spelunk. DodoSSH can use @@ -671,8 +697,9 @@ its host if it has one and back into the box if it does not. **A connected phone shows one bar and then the terminal.** The header, the session strip and the bottom bar are collapsed while a shell is up, and a single 35-pixel row replaces them: back on the left, the sessions as pills, and a `+` on the right offering the three connections this application can make — a -shell, a host's files over SFTP, or a bucket. The system back gesture does what the arrow does, and lowers -that menu first if it is open. +shell, a host's files over SFTP, or a bucket. Beside the `+`, for a tab whose host has port forwards, a `⇄` +opens them; see below. The system back gesture does what the arrow does, and lowers either sheet first if +one is open. Widening the rail moved the desktop window's minimum from `880x560` to `1016x574`, which leaves every screen exactly the width it was designed against. @@ -740,10 +767,25 @@ What it does *not* yet do is use the extra width inside a screen: the host list size. Two columns needs the row model to change — the list is headings and hosts in one sequence, and a heading has to span — which is shared with the desktop and is its own piece of work. -**Port forwarding is not built anywhere**, and the phone's More screen says so in a paragraph rather than -leaving a gap. The v2 design draws a whole screen for it; nothing in the SSH layer forwards anything, so -every control on that screen would have had no effect. See -[`docs/design-import-gaps.md`](docs/design-import-gaps.md). +**Port forwarding runs here too, and is edited on the desktop.** A host's forwards start with every +terminal the phone opens to it, as they do on the desktop. The terminal's bar grows a `⇄` for a tab that has +any, carrying the number listening in green, or turning red when one could not start; it opens a sheet +listing the tab's forwards with their route, their state, the reason a failed one gave, and a tap on each to +switch it off or on — the desktop sidebar's FORWARDS section, over the same rows. The v2 design's whole +FORWARDING screen is not drawn, for the reason the desktop has none: a forward belongs to a host and runs +with a session, and both already have a place. What the phone has not got is an editor for them — its host +form has no PORT FORWARDS section — so they are added on the desktop and arrive with the host; the More +screen says so in a paragraph. + +Forwards ride the **keep-alive** a shell already has. A forward runs over its tab's connection, so the +foreground service that holds a shell open in the background is holding its forwards open too, and nothing +else needs to: the service is never started for a forward alone, because there is never a listening forward +without a live shell. What forwards change is the notification, which counts them beside the shells — +`1 shell connected · 2 ports forwarded` — because a phone in a pocket holding ports open is something its +owner should be able to see. Local and SOCKS forwards listen on the phone's own loopback, which every app on +the phone can reach; that is what makes one useful here — a database client on the phone reaching a server +behind the remote — and it is the same exposure the desktop's localhost has to its own processes. See +[Forwarding ports](#forwarding-ports) and [`docs/design-import-gaps.md`](docs/design-import-gaps.md). It is deliberately **not** in `DodoSSH.slnx`. Putting it there would make the `android` workload and a full Android SDK a prerequisite of `dotnet build DodoSSH.slnx` for everybody; it has its own CI job instead, which diff --git a/docs/android-port.md b/docs/android-port.md index 31c7e51..8524354 100644 --- a/docs/android-port.md +++ b/docs/android-port.md @@ -631,7 +631,8 @@ go at 360dp: for. `ShellScreen` gained `More` and `Buckets`. The palette went blue and the two heads moved together, because it is shared. What v2 asked for and did not get is in [`docs/design-import-gaps.md`](design-import-gaps.md) — the short version being that port forwarding is - named as absent rather than drawn, because nothing in the SSH layer forwards anything. + named rather than drawn as a screen: it has since been built, and this head runs a host's forwards with + every terminal and lists them in a sheet off the terminal's bar. It cannot edit them yet. **The bar has since gone from four entries to three.** The keychain joined the hub, which is now called SETTINGS rather than MORE and wears a gear: a bottom bar is for the places a session moves between, and diff --git a/docs/design-import-gaps.md b/docs/design-import-gaps.md index 11c3bb0..03f34a8 100644 --- a/docs/design-import-gaps.md +++ b/docs/design-import-gaps.md @@ -39,8 +39,8 @@ the chrome, hosts and terminals, file transfer, the vault, teams, and preference > > | v2 element | What ships instead | > | --- | --- | -> | The **FORWARDING** screen: local/remote/dynamic rules, toggles, bytes transferred | **Nothing, said out loud.** `ISshConnection` offers `OpenShellAsync` and nothing else, so there is no tunnel for a rule to run through; `SyncEntityType.PortForward = 9` is still reserved and still unused. The MORE screen carries a paragraph naming the absence, for the reason the desktop keeps VAULTS in its rail. | -> | `23 ms · fwd 5432` on the terminal's connection line | ◆ **The line is gone, and what was real on it moved.** There was never an RTT to draw — SSH.NET measures none — and nothing forwards anything, so what shipped was the account and endpoint actually dialled. In v3 a connected phone draws one 35-pixel bar and then the terminal, so a second 36-pixel row naming the machine is exactly the chrome that surface exists to give back: the address is on the connecting card, where it is read before anything has answered, and the shell's own prompt says it afterwards. The two text-size buttons that shared the line are pinned at the end of the accessory row, outside its scroller, which is what the line was protecting them from. | +> | The **FORWARDING** screen: local/remote/dynamic rules, toggles, bytes transferred | **Built, as a sheet off the terminal rather than a screen, and running on the phone as on the desktop.** A host's forwards start with every terminal opened to it on either head — `MainWindowViewModel.StartForwards` no longer skips Android. The terminal's bar draws a `⇄` for a tab that has forwards, with the listening count in green and the glyph in the danger colour when one failed, and it raises a sheet (`IsForwardsSheetOpen`) listing `SelectedTab.Forwards` with notation, route, state word, failure reason and a tap-to-toggle per row — the desktop sidebar's FORWARDS section over the same rows. Not a screen, for the desktop's reason: a forward belongs to a host and runs with a session. **Two gaps remain.** The phone's host form has no PORT FORWARDS section, so forwards are edited on the desktop and arrive with the host; the MORE screen's paragraph, now tagged IN THE TERMINAL, says so. And byte counts are built on neither head: SSH.NET keeps none. The keep-alive notification counts listening forwards beside the shells they run over, and is never started for a forward alone, since a forward cannot outlive its shell. | +> | `23 ms · fwd 5432` on the terminal's connection line | ◆ **The line is gone, and what was real on it moved.** There was never an RTT to draw — SSH.NET measures none — and nothing forwarded anything at the time (forwards now have the `⇄` in the bar above; see the row before this one), so what shipped was the account and endpoint actually dialled. In v3 a connected phone draws one 35-pixel bar and then the terminal, so a second 36-pixel row naming the machine is exactly the chrome that surface exists to give back: the address is on the connecting card, where it is read before anything has answered, and the shell's own prompt says it afterwards. The two text-size buttons that shared the line are pinned at the end of the accessory row, outside its scroller, which is what the line was protecting them from. | > | `ED25519` badge and `SHA256:kQ9f…Zw2M` on every keychain card | `Detail`, which is what is genuinely known *about* an item. Unchanged from the first import: no algorithm field, no fingerprint, and computing either means parsing armour the type stores verbatim. | > | An `agent` chip on a key | Omitted. There is no agent of any kind — see the first import's Vault section. | > | Snippet cards footed with `edge-eu-1 · today 10:58 · exit 0` | The command and its badge. Nothing records where a snippet ran, when, or what it returned; the shell it is typed into never reports back. | @@ -78,7 +78,7 @@ the chrome, hosts and terminals, file transfer, the vault, teams, and preference > > | v2 element | What ships instead | > | --- | --- | -> | The **FORWARDING** screen, the `⇄ N forwards` chip on the session strip, and the host detail's FORWARDING card | Nothing, as on the phone and for the same reason. Three surfaces for a feature the SSH layer does not have. | +> | The **FORWARDING** screen, the `⇄ N forwards` chip on the session strip, and the host detail's FORWARDING card | **Built, as three smaller things.** Forwards are rules on the host, edited in its drawer under PORT FORWARDS and shown read-only on its detail pane; they run per terminal, listed with a switch each under FORWARDS in the session sidebar; and the session status bar counts the listening ones as `⇄ N FORWARDS`. No screen of their own, because a forward is not a thing you visit — it belongs to a host and runs with a session, and both of those already have a place. | > | The host detail's 2×2 card grid — IDENTITY fingerprint, TAGS, LAST SESSION | Not drawn as a grid. There is no fingerprint on a stored key and no last-used timestamp at any layer, so two of the three cards have nothing behind them. Tags are real and are drawn — as chips on the row and a picker in the editor, which is where they are used rather than in a card beside two empty ones. | > | The keychain detail's public-key block, USED BY hosts, and **Rotate** | The detail pane as it stands. Copy public key is real and already there; rotation is not a thing this client can do. | > | Logs: **FOLLOW**, the severity chips, and `tailing via ssh · journald + files` | The two logs it really has. Same as the phone — these are synced audit records, not a tail, and the footer sentence describes a feature rather than the screen. | @@ -171,7 +171,7 @@ the chrome, hosts and terminals, file transfer, the vault, teams, and preference > | The **Backspace / Default** row | Omitted, and it is the one row on this list that could now be built cheaply — a setting, an opcode and a control, the way the text size was. What it lacks is a reason: which byte backspace sends is a fact about the remote's `stty`, so a client-side switch fixes a mismatch by hiding it. See the Preferences section. | > | The **chevron beside the vault name** | The name alone, and the move behind the pane's ⋯ menu instead. A host *can* now be moved between vaults, so the gap is no longer that there is nothing to offer — it is that a chevron on a subtitle implies an edit, and this is not one: the two vaults are encrypted under different keys, so it is a re-seal into one and a tombstone in the other, the host takes a new id, and its group and tags stay behind. A control that implied "just change this field" would be describing something else. Where a *new* host goes is still asked in the host editor, as a picker beside the name. A group moves too, from its card's right-click menu, and takes its nested groups and every host filed under them; keys, passwords and buckets take theirs from the keychain screen's standing picker and cannot be moved yet. | > | **Show more ⌄** | Not drawn as a disclosure. What it would hide — notes, the relay switch, forgetting the host key — is in the editor, one press away, and a second fold inside a pane that already scrolls is a second place for a field to be missing from. | -> | **Port Forwarding** in the sidebar | Nothing, for the third time in this document. | +> | **Port Forwarding** in the sidebar | No rail entry; see the FORWARDING row above for where forwards are instead. | > | The host grid's toolbar avatar, share and tag-filter controls | Omitted, as in v3 and for the same reasons. | > > ## The desktop's v5 — hosts, restyled, and two reversals @@ -295,7 +295,7 @@ the chrome, hosts and terminals, file transfer, the vault, teams, and preference > | v5b element | What ships instead | > | --- | --- | > | The host header's OS label (`Ubuntu 24.04 LTS`) and latency reading (`12 ms`) | Neither. The client does not know the remote's OS and nothing measures round-trip time — the same two absences the terminal pane header already recorded before this pass, carried into the new header rather than reopened. | -> | A **Port forward** button on the host header | Omitted. The feature does not exist; see the FORWARDING rows earlier in this document. | +> | A **Port forward** button on the host header | **Add forward**, at the foot of the session sidebar's FORWARDS list, which opens the host's editor the way Pin folder does. The header it would have sat on has since moved into that sidebar. | > | The tab and status dots' third, amber state | ◆ **Still two states**, for the reason the hosts screen's own dot has stayed two states since v3: green means a shell is open (or a session is connected), grey means it is not, and nothing here pings a host to justify a third colour meaning "reachable but not connected". | > | The status bar's negotiated cipher, host-key algorithm and key/credential name | ◆ **Shipped, on both surfaces, with three honest deviations.** `ISshConnection` and `ISftpSession` now both carry `Cipher` — the server-to-client algorithm off SSH.NET's own `ConnectionInfo.CurrentServerEncryption`, captured once at construction because a rekey is not an event SSH.NET raises — and `TerminalWorkspace.GetSessionFacts` hands the cipher and the host key's algorithm back to the shell the moment a session opens; `VaultViewModel.TryBuildAuthentication` now threads the authenticating key's or credential's own `Label` into `HostAuthentication.IdentityLabel`, all the way to `MainWindowViewModel`'s surface-aware `SessionCipher`, `SessionHostKeyAlgorithm` and `SessionIdentityLabel`, composed into one `SessionIdentityText` run for the status bar. Three deviations from the mock, not omissions: the algorithm prints exactly as negotiated (`ssh-ed25519`), not the design's shortened `ed25519`, because trimming it would be an edit to a string this client did not choose; the run is plain text rather than the design's clickable element, because there is no pin-details modal for a session that is already open, and drawing a click target for a screen that does not exist would itself be a fabrication; and a typed-password session — nothing filed in the keychain to name — shows the host-key algorithm alone, with no `·` after it, because there is no item behind the dot. | > | S3 dimmed in the design's own switcher | **Enabled.** The mock leaves S3 as future work; this application already has bucket browsing, so SSH, SFTP and S3 are a true three-way segment, wired to `IsSshShowing`, `IsTransfersShowing` and `IsBucketsShowing` exactly alike. | @@ -392,9 +392,10 @@ sharing that would rest on them — or a decision recorded here not to build the Three facts explain nearly every row below. -**An SSH connection here opens exactly one channel.** `ISshConnection` offers `OpenShellAsync` and nothing -else (`src/DodoSSH.Client.Ssh/SshConnection.cs`). No port forwarding, no ProxyJump. That one fact removes -the `FORWARDS` chip, the status bar's port list, and every `via bastion-eu` in the design. +**An SSH connection here opens a shell, and forwards beside it.** `ISshConnection` offers `OpenShellAsync` +and `StartForwardAsync` (`src/DodoSSH.Client.Ssh/SshConnection.cs`). There is still no ProxyJump, which is +what removes every `via bastion-eu` in the design. Port forwarding arrived later than most of what this +document accounts for, and the rows about it now say where it went. It used to remove the file-transfer screen too. M2 did not lift the restriction — it worked around it: file transfer is a *separate connection* rather than a second channel, because SSH.NET's `SftpClient` owns @@ -461,8 +462,10 @@ configuration, a server kind that refuses every plaintext field, a codec, a merg same way, for the same cost, and is the clearest case the reservation ever paid for: a whole item kind, a table, a migration and an AAD arm, with `PublicAPI.Unshipped.txt` untouched. -`HostTag`, `HostCredential` and `PortForward` are still reserved and still unused — and `HostTag` is now -reserved *against a decision* rather than pending one. Which hosts wear a tag is a set of ids inside each +`HostTag`, `HostCredential` and `PortForward` are still reserved and still unused — and `HostTag` and +`PortForward` are now reserved *against a decision* rather than pending one. Port forwards shipped inside the +host's payload, for the reason `RelayTicketRequest.PortForwardId` already assumed and the one below gives for +tags. Which hosts wear a tag is a set of ids inside each host's payload, not a join: the one thing a join buys over that is two machines tagging the same host without either losing, and the client's per-key three-way merge already gives it. A second item type would be a second table and a second pass through every rule in this file, for nothing. @@ -509,7 +512,7 @@ field cannot be removed and stays as a permanently refused member; `SyncEndpoint | `VAULT SYNCED 11:02` | client-session | `StoredSyncState.LastPulledAt`/`LastPushedAt` are persisted, but `VaultSession` exposes the store as `internal`. A property away. | Omitted. The one honest sync fact — the outbox depth — is in the titlebar and the status bar. | | `⌘K` command palette running commands | client-domain | A snippet or saved-command item type (`SyncEntityType.Snippet = 8` is reserved). | Ctrl+K opens a real host search that connects on Enter. The box says "search hosts", not "search hosts · run command". | | Status bar `· via bastion-eu` | client-ssh | Jump-host execution. See below. | Omitted. | -| Status bar port forwards | client-ssh | Port forwarding. See below. | Omitted. | +| Status bar port forwards | client-ssh | — | **Shipped** on the session status bar as a count of listening forwards; the ports themselves are in the session sidebar. | | Status bar `sftp · 2 transfers` | client-app | Nothing now — file transfer is built. What is missing is the count reaching the status bar, which is a screen away from where the queue lives. | Omitted from the status bar. The queue itself is on the FILES screen, with a row per transfer. | | Status bar `locks in 09:41` | client-app | An idle auto-lock. See preferences below. | Omitted. | | IBM Plex Mono / IBM Plex Sans | ui | Shipping the font files as `AvaloniaResource` and registering them. The design loads them from Google Fonts, which a desktop app cannot. | **Shipped, since v5, and not IBM Plex.** The v5 design asked for Montserrat, JetBrains Mono and Material Icons instead, and all three are embedded as `AvaloniaResource` in the Shell project rather than requested by name and left to whatever the machine happens to have. Montserrat is the default sans, with Inter — the earlier substitution — kept only as its fallback; `MonoFont` puts JetBrains Mono first, ahead of the system-monospace stack the terminal still falls back to. Named once each in `Program.cs` and `Palette.axaml`, so a later design's fonts are still a one-line swap. | @@ -533,7 +536,7 @@ caption buttons and window title drawn on top of the application's own — two s | Per-host status dot, three colours | client-ssh | The amber state would mean "reachable but not connected", and nothing here ever probes a host. | Two states, both real: green when a terminal is open on that host, grey when not. | | `· ⤷ bastion-eu` in the host subtitle | client-ssh | **Jump hosts are data-only.** `HostSecret.JumpHostIds` is a `JumpChain` that is stored, encrypted, synced and three-way merged — and nothing reads it at connect time. `SshConnectionRequest` carries one host. | Omitted. The stored chain is preserved untouched by every edit. | | `SPLIT ⌘D` and side-by-side panes | client-ssh + ui | The renderer stacks panes and shows one (`terminal.css`: `.pane { position:absolute; inset:0; display:none }`). Tiling needs a real pane geometry and a splitter. | Omitted. Tabs ship instead, over the same one-WebView multiplexing. | -| `⇄ FORWARDS · 2` | client-ssh | Port forwarding. `SyncEntityType.PortForward = 9` is reserved; nothing in the SSH layer forwards anything. | Omitted. | +| `⇄ FORWARDS · 2` | client-ssh | — | **Shipped**, as `⇄ 2 FORWARDS` on the session status bar, counting only forwards actually listening. | | `SNIPPETS` panel, `↵` to run | client-domain | A snippet item type (`Snippet = 8`, reserved). | **Shipped**, as a screen rather than a panel. `↵` is per snippet and off by default: inserting types the command at the prompt and stops, because nothing here can tell whether the terminal is at a prompt at all. | | Broadcast to all panes (`⌥↵`) | client-ssh | Input is routed strictly by session id in `TerminalDataPlane.Dispatch`; there is no fan-out. Needs splits first. | Omitted. | | Pane header `24ms` | client-ssh | Round-trip measurement. SSH.NET offers no RTT API. | Omitted. | diff --git a/src/DodoSSH.Client.Android/App.axaml.cs b/src/DodoSSH.Client.Android/App.axaml.cs index ad53d6b..8d0fdb8 100644 --- a/src/DodoSSH.Client.Android/App.axaml.cs +++ b/src/DodoSSH.Client.Android/App.axaml.cs @@ -132,7 +132,8 @@ public sealed partial class DodoSshApp : Avalonia.Application var keepAlive = new SessionKeepAlive( workspace, activeTransfers: () => viewModel.Transfers.ActiveTransfers, - holdsFileSession: () => viewModel.Transfers.HasLiveFileSession); + holdsFileSession: () => viewModel.Transfers.HasLiveFileSession, + listeningForwards: () => viewModel.ListeningForwardCount); // The other end of the same wire: the workspace announces its own sessions ending, and the queue // announces transfers appearing and finishing, and a Files session connecting or disconnecting. @@ -148,6 +149,11 @@ public sealed partial class DodoSshApp : Avalonia.Application // in the first place, which was precisely the promise this whole arrangement exists to keep. viewModel.TerminalSessionOpened += (_, _) => keepAlive.Refresh(); + // A forward starting, stopping or failing changes the notification's wording and nothing else — it + // rides a shell's connection, so the shell is already what holds the service up. The user is still + // entitled to know a phone in their pocket is holding ports open. + viewModel.ForwardsChanged += (_, _) => keepAlive.Refresh(); + keepAlive.Refresh(); } diff --git a/src/DodoSSH.Client.Android/Platform/SessionForegroundService.cs b/src/DodoSSH.Client.Android/Platform/SessionForegroundService.cs index 216efb1..5413f2b 100644 --- a/src/DodoSSH.Client.Android/Platform/SessionForegroundService.cs +++ b/src/DodoSSH.Client.Android/Platform/SessionForegroundService.cs @@ -141,6 +141,11 @@ internal sealed class SessionForegroundService : Service /// Shells with a live channel behind them. /// Transfers still moving bytes. /// Whether the Files screen holds a live, idle SFTP connection. + /// + /// Port forwards listening over those shells. Named in the summary and nothing else: a forward rides a + /// shell's connection, so it cannot be the reason the process has to stay up — see + /// . + /// /// /// Three outcomes, not the two a plain start-or-stop would have. Nothing live stops the service, as /// always. Something live and the service not yet running starts it. Something live and the service @@ -152,7 +157,7 @@ internal sealed class SessionForegroundService : Service /// which needs no foreground-start permission at all. A foregrounded refresh still prefers a real /// start even over a service that looks like it is running; the inline remark below is why. /// - public static void Reconcile(int liveSessions, int activeTransfers, bool holdsFileSession) + public static void Reconcile(int liveSessions, int activeTransfers, bool holdsFileSession, int listeningForwards) { var context = PhoneEnvironment.Require(); @@ -165,7 +170,7 @@ internal sealed class SessionForegroundService : Service // The summary says what is actually held, counted rather than generic — the same principle the // delete confirmations follow. "DodoSSH is running" would tell the user nothing they could act on. - var summary = Summarise(liveSessions, activeTransfers, holdsFileSession); + var summary = Summarise(liveSessions, activeTransfers, holdsFileSession, listeningForwards); // In-place only while backgrounded, where it is the only legal move. Foregrounded, a real start is // always allowed and is preferred even when `running` says the service is up: a disconnect followed @@ -255,15 +260,22 @@ internal sealed class SessionForegroundService : Service activity.RequestPermissions([postNotifications], NotificationPermissionRequestCode)); } - private static string Summarise(int liveSessions, int activeTransfers, bool holdsFileSession) + private static string Summarise(int liveSessions, int activeTransfers, bool holdsFileSession, int listeningForwards) { - var parts = new List(3); + var parts = new List(4); if (liveSessions > 0) { parts.Add(liveSessions == 1 ? "1 shell connected" : $"{liveSessions} shells connected"); } + // Beside the shells they run over, and only while one is: a count left over from a session that has + // just ended would be a receipt for ports nothing holds open any more. + if (liveSessions > 0 && listeningForwards > 0) + { + parts.Add(listeningForwards == 1 ? "1 port forwarded" : $"{listeningForwards} ports forwarded"); + } + if (holdsFileSession) { parts.Add("Files connected"); diff --git a/src/DodoSSH.Client.Android/Platform/SessionKeepAlive.cs b/src/DodoSSH.Client.Android/Platform/SessionKeepAlive.cs index 374d6bc..07e2c10 100644 --- a/src/DodoSSH.Client.Android/Platform/SessionKeepAlive.cs +++ b/src/DodoSSH.Client.Android/Platform/SessionKeepAlive.cs @@ -24,12 +24,20 @@ namespace DodoSSH.Client.Android.Platform; /// looked, to this class, exactly like nothing being open at all. holdsFileSession below is that /// gap closed, read the same way the other two facts are: asked, not cached. /// +/// +/// Listening port forwards are a fourth fact, and a different kind: they only change what the notification +/// says. A forward runs over a shell's connection and stops with it, so there is never a listening forward +/// without a live shell already holding the service up — and a count that could start the service on its own +/// would only ever be wrong, in the moment a forward's Stopped had not been heard yet. What they add is the +/// receipt: the user is entitled to know the phone is holding ports open, as well as shells. +/// /// internal sealed class SessionKeepAlive : IDisposable { private readonly TerminalWorkspace workspace; private readonly Func activeTransfers; private readonly Func holdsFileSession; + private readonly Func listeningForwards; /// The live shells. /// @@ -40,11 +48,20 @@ internal sealed class SessionKeepAlive : IDisposable /// Whether the Files screen holds a live SFTP connection with nothing moving on it — the idle-but- /// connected case a transfer count alone would miss. See TransfersViewModel.HasLiveFileSession. /// - public SessionKeepAlive(TerminalWorkspace workspace, Func activeTransfers, Func holdsFileSession) + /// + /// How many port forwards are listening, across every tab. For the notification's wording only; see the + /// class remarks. See MainWindowViewModel.ListeningForwardCount. + /// + public SessionKeepAlive( + TerminalWorkspace workspace, + Func activeTransfers, + Func holdsFileSession, + Func listeningForwards) { this.workspace = workspace; this.activeTransfers = activeTransfers; this.holdsFileSession = holdsFileSession; + this.listeningForwards = listeningForwards; // Raised on whatever thread the workspace announced from — a continuation of the ended run, or the // closer's own — which is fine: starting and stopping a service is a binder call and needs no @@ -56,14 +73,15 @@ internal sealed class SessionKeepAlive : IDisposable /// Re-reads the counts and starts or stops the service to match. /// - /// Called after anything that could change any of the three facts — opening a shell, closing a tab, a - /// transfer finishing, a Files connection opening or closing. Calling it when nothing changed is free: + /// Called after anything that could change any of the facts — opening a shell, closing a tab, a transfer + /// finishing, a Files connection opening or closing, a forward starting or stopping. Calling it when nothing changed is free: /// reconciling to the state it is already in is either a redundant startForegroundService — or, /// now, a redundant notification post — on a running service, or a stopService on a stopped one, /// and Android treats all of those as no-ops. /// public void Refresh() => - SessionForegroundService.Reconcile(workspace.LiveSessionCount, activeTransfers(), holdsFileSession()); + SessionForegroundService.Reconcile( + workspace.LiveSessionCount, activeTransfers(), holdsFileSession(), listeningForwards()); /// public void Dispose() @@ -72,7 +90,7 @@ internal sealed class SessionKeepAlive : IDisposable // The notification goes with the composition root. Leaving it up over a process that is shutting // down is how an SSH client acquires a reputation for a notification you cannot get rid of. - SessionForegroundService.Reconcile(0, 0, false); + SessionForegroundService.Reconcile(0, 0, false, 0); } private void OnSessionEnded(object? sender, TerminalSessionEndedEventArgs e) => Refresh(); diff --git a/src/DodoSSH.Client.Android/Views/MoreScreen.axaml b/src/DodoSSH.Client.Android/Views/MoreScreen.axaml index 0b98980..ee7d73b 100644 --- a/src/DodoSSH.Client.Android/Views/MoreScreen.axaml +++ b/src/DodoSSH.Client.Android/Views/MoreScreen.axaml @@ -189,14 +189,18 @@ ◆ Port forwarding, stated rather than drawn. The v2 design gives this hub a sixth entry and a full screen behind it: local, remote and dynamic - rules with toggles and byte counters. Nothing in this application forwards anything — ISshConnection - offers OpenShellAsync and nothing else — so every switch on that screen would be a control with no - effect and every counter a number with no source. + rules with toggles and byte counters. Forwarding is built and runs here as on the desktop — a host's + forwards start with every terminal opened to it — but it is not a destination, so it has no row. A + forward belongs to a host and runs with a session, and on this head the session's forwards are a + sheet off the terminal's bar: the ⇄ button, drawn for a tab that has some, lists them with a switch + each. See MainWindowViewModel.IsForwardsSheetOpen. + + What the phone lacks is the editor: its host form has no PORT FORWARDS section yet, so forwards are + added on the desktop and arrive here with the host. The byte counters are not built anywhere; + SSH.NET keeps no count to draw. It is a paragraph here rather than a missing row for the reason the desktop keeps TEAMS in its rail: - a destination that is quietly absent reads as a product that never intended it, and this one is - intended. SyncEntityType.PortForward = 9 is reserved on the wire and unused. See - docs/design-import-gaps.md. + a place somebody looks for a feature should say where it went. See docs/design-import-gaps.md. --> @@ -206,11 +210,11 @@ - + + Text="Forwards saved on a host start with every terminal opened to it, here as on the desktop. The ⇄ button in the terminal's bar lists them, with a switch each and the reason one could not start. They are added and edited in the desktop app's host editor; this phone runs them but has no editor for them yet." /> diff --git a/src/DodoSSH.Client.Android/Views/PhoneShell.axaml.cs b/src/DodoSSH.Client.Android/Views/PhoneShell.axaml.cs index 45e6be9..466b052 100644 --- a/src/DodoSSH.Client.Android/Views/PhoneShell.axaml.cs +++ b/src/DodoSSH.Client.Android/Views/PhoneShell.axaml.cs @@ -579,7 +579,7 @@ internal sealed partial class PhoneShell : UserControl /// /// The connect menu is a second such guard, and it matters more than the first. A terminal now /// fills the screen — no header, no bottom bar — so while that menu is up this gesture is the only way - /// off it other than the scrim and CANCEL. It is checked before the terminal is dismissed for the + /// off it other than the scrim and CANCEL. The forwards sheet, raised from the same bar, shares the guard. It is checked before the terminal is dismissed for the /// reason it is drawn over it: back takes the topmost thing, and dismissing the surface underneath a /// menu would take two, neither of them the one being looked at. /// @@ -598,13 +598,12 @@ internal sealed partial class PhoneShell : UserControl return; } - // The connect menu, which is raised from the terminal's own bar and is the topmost thing the phone - // draws while it is up. Ahead of the editors below because it is nearer, and ahead of leaving the - // terminal because a gesture that dismissed the surface underneath a menu would close two things at - // once — and the one the user was looking at would not be either of them. - if (current.IsConnectSheetOpen) + // The connect menu or the forwards sheet, which are raised from the terminal's own bar and are the + // topmost thing the phone draws while one is up. Ahead of the editors below because they are nearer, + // and ahead of leaving the terminal because a gesture that dismissed the surface underneath a sheet + // would close two things at once — and the one the user was looking at would not be either of them. + if (TryCloseATerminalSheet(current)) { - current.CloseConnectSheetCommand.Execute(null); e.Handled = true; return; } @@ -647,6 +646,31 @@ internal sealed partial class PhoneShell : UserControl } } + /// + /// Lowers the connect menu or the forwards sheet, whichever is up over the terminal. + /// + /// Whether one was closed, and so whether back has been spent. + /// + /// Both are raised from the terminal's bar and the scrim under each covers that bar, so at most one is + /// ever open; the order below decides nothing. + /// + private static bool TryCloseATerminalSheet(MainWindowViewModel current) + { + if (current.IsConnectSheetOpen) + { + current.CloseConnectSheetCommand.Execute(null); + return true; + } + + if (current.IsForwardsSheetOpen) + { + current.CloseForwardsSheetCommand.Execute(null); + return true; + } + + return false; + } + /// /// Lowers whatever the hosts screen has raised over its list, topmost first. /// diff --git a/src/DodoSSH.Client.Android/Views/TerminalScreen.axaml b/src/DodoSSH.Client.Android/Views/TerminalScreen.axaml index a511cae..1827425 100644 --- a/src/DodoSSH.Client.Android/Views/TerminalScreen.axaml +++ b/src/DodoSSH.Client.Android/Views/TerminalScreen.axaml @@ -110,6 +110,34 @@ + + + + + + + @@ -139,7 +167,7 @@ job — see the remark on the header in PhoneShell.axaml. --> - + - + + + + + + + + + + + + + @@ -785,6 +813,84 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/DodoSSH.Client.App/Views/SessionSidebar.axaml.cs b/src/DodoSSH.Client.App/Views/SessionSidebar.axaml.cs index 44dd362..a1b0972 100644 --- a/src/DodoSSH.Client.App/Views/SessionSidebar.axaml.cs +++ b/src/DodoSSH.Client.App/Views/SessionSidebar.axaml.cs @@ -19,6 +19,10 @@ internal sealed partial class SessionSidebar : UserControl internal static readonly StyledProperty ShowsSnipsProperty = AvaloniaProperty.Register(nameof(ShowsSnips)); + /// Whether the FORWARDS section draws. The terminal's own, for the reason SNIPS is. + internal static readonly StyledProperty ShowsForwardsProperty = + AvaloniaProperty.Register(nameof(ShowsForwards)); + public SessionSidebar() => InitializeComponent(); internal bool ShowsSnips @@ -26,4 +30,10 @@ internal sealed partial class SessionSidebar : UserControl get => GetValue(ShowsSnipsProperty); set => SetValue(ShowsSnipsProperty, value); } + + internal bool ShowsForwards + { + get => GetValue(ShowsForwardsProperty); + set => SetValue(ShowsForwardsProperty, value); + } } diff --git a/src/DodoSSH.Client.App/Views/SessionStatusBar.axaml b/src/DodoSSH.Client.App/Views/SessionStatusBar.axaml index c6b3d11..90e1fc1 100644 --- a/src/DodoSSH.Client.App/Views/SessionStatusBar.axaml +++ b/src/DodoSSH.Client.App/Views/SessionStatusBar.axaml @@ -76,6 +76,13 @@ + + diff --git a/src/DodoSSH.Client.App/Views/StatusBar.axaml b/src/DodoSSH.Client.App/Views/StatusBar.axaml index 212a897..e2fa621 100644 --- a/src/DodoSSH.Client.App/Views/StatusBar.axaml +++ b/src/DodoSSH.Client.App/Views/StatusBar.axaml @@ -9,8 +9,9 @@ The design puts five things here: the active host and the bastion it is reached through, the open port forwards, the SFTP transfer count, an auto-lock countdown, and the last sync time. Four of those five - have nothing behind them — there is no jump-host routing, no port forwarding, no file transfer and no - idle auto-lock — so what is left is the active session, the vault's last message, and the shortcut hint. + had nothing behind them when this was written — there was no jump-host routing, no port forwarding, no + file transfer and no idle auto-lock — so what is left is the active session, the vault's last message, + and the shortcut hint. Forwards have since been built, and are counted on SessionStatusBar instead. The last message is the load-bearing one, and it is why this bar is worth having at all: the vault's status line used to be a cramped label in the account bar, and everything the application has to say diff --git a/src/DodoSSH.Client.Domain/HostSecret.cs b/src/DodoSSH.Client.Domain/HostSecret.cs index e74cec9..fcf7ec3 100644 --- a/src/DodoSSH.Client.Domain/HostSecret.cs +++ b/src/DodoSSH.Client.Domain/HostSecret.cs @@ -45,6 +45,13 @@ public sealed record HostSecret : IVaultSecret /// public const int MaxPinnedPaths = 32; + /// The most port forwards a host may carry. + /// + /// Each one is a listener opened with every terminal to the host, so the bound is on what one connect + /// may open as much as on the payload. + /// + public const int MaxPortForwards = 16; + /// Display name. The only name this host has anywhere. public required string Label { get; init; } @@ -223,6 +230,15 @@ public sealed record HostSecret : IVaultSecret /// public PinnedPathList PinnedPaths { get; init; } = PinnedPathList.Empty; + /// + /// The port forwards started with every terminal opened to this host. + /// + /// + /// Here rather than in an item of their own; see . Merged per forward, so two + /// machines each adding one both keep theirs, and two editing the same one is a recorded conflict. + /// + public PortForwardList PortForwards { get; init; } = PortForwardList.Empty; + /// /// The group this host is filed under, or null for none. /// @@ -339,10 +355,32 @@ public sealed record HostSecret : IVaultSecret } return PinnedPathsAreStorable(out reason) + && PortForwardsAreStorable(out reason) && ReferencesAreStorable(out reason) && RouteIsStorable(out reason); } + /// Checks the forwards saved on this host, each on its own terms and the list as a whole. + private bool PortForwardsAreStorable([NotNullWhen(false)] out string? reason) + { + if (PortForwards.Count > MaxPortForwards) + { + reason = $"A host cannot carry more than {MaxPortForwards} port forwards."; + return false; + } + + foreach (var forward in PortForwards) + { + if (!forward.TryValidate(out reason)) + { + return false; + } + } + + reason = null; + return true; + } + /// Checks the Spelunk route, if there is one, and that it is the only route. private bool RouteIsStorable([NotNullWhen(false)] out string? reason) { diff --git a/src/DodoSSH.Client.Domain/HostSecretCodec.cs b/src/DodoSSH.Client.Domain/HostSecretCodec.cs index 76d4a1a..4c369e5 100644 --- a/src/DodoSSH.Client.Domain/HostSecretCodec.cs +++ b/src/DodoSSH.Client.Domain/HostSecretCodec.cs @@ -112,6 +112,13 @@ public static class HostSecretCodec /// public const int PinnedPathsSchemaVersion = 7; + /// The version that introduced . + /// + /// Its own version for the reason gives: a host can carry + /// forwards without pinning anything, and should stay editable on a build that knows 7 until it does. + /// + public const int PortForwardsSchemaVersion = 8; + /// The version that introduced . /// /// @@ -122,10 +129,11 @@ public static class HostSecretCodec /// /// /// Additive, like every version but : the route is one more - /// property, and a build that does not know it skips it and still reads a valid host. + /// property, and a build that does not know it skips it and still reads a valid host. It follows + /// , whose forwards reached main first. /// /// - public const int SpelunkSchemaVersion = 8; + public const int SpelunkSchemaVersion = 9; /// The highest schema version this build can write. /// @@ -179,6 +187,10 @@ public static class HostSecretCodec // as it did before this field existed. PinnedPaths = host.PinnedPaths.Count == 0 ? null : [.. host.PinnedPaths], + // Last, and null when empty, for the reason both fields above give. + PortForwards = host.PortForwards.Count == 0 + ? null + : [.. host.PortForwards.Select(PortForwardDocument.From)], // Last of all, and absent for a host that is dialled, so every host without a route encodes // exactly as it did before routes existed. Spelunk = host.Spelunk is { } route @@ -264,6 +276,11 @@ public static class HostSecretCodec version = Math.Max(version, PinnedPathsSchemaVersion); } + if (host.PortForwards.Count > 0) + { + version = Math.Max(version, PortForwardsSchemaVersion); + } + if (host.Spelunk is not null) { version = Math.Max(version, SpelunkSchemaVersion); @@ -326,6 +343,11 @@ public static class HostSecretCodec return false; } + if (!TryBuildForwards(parsed.PortForwards, out var forwards)) + { + return false; + } + var candidate = new HostSecret { Label = parsed.Label ?? string.Empty, @@ -348,6 +370,7 @@ public static class HostSecretCodec PinnedPaths = PinnedPathList.Create(parsed.PinnedPaths ?? []), + PortForwards = forwards, Spelunk = RouteFrom(parsed.Spelunk), }; @@ -360,6 +383,45 @@ public static class HostSecretCodec return true; } + /// + /// A missing id, kind or port is a refusal rather than a default. Each would be invented here — a fresh + /// id the other machines do not have, a Local nobody chose — and an invented forward is one the + /// next save writes back as though somebody had asked for it. + /// + private static bool TryBuildForwards( + PortForwardDocument?[]? parsed, + [NotNullWhen(true)] out PortForwardList? forwards) + { + forwards = PortForwardList.Empty; + + if (parsed is null) + { + return true; + } + + var rules = new List(parsed.Length); + + foreach (var entry in parsed) + { + if (entry?.Id is not { } id || entry.Kind is not { } kind || entry.BindPort is not { } bindPort) + { + return false; + } + + rules.Add(new PortForwardRule + { + Id = id, + Kind = (PortForwardKind)kind, + BindPort = bindPort, + TargetHost = entry.TargetHost, + TargetPort = entry.TargetPort, + }); + } + + forwards = PortForwardList.Create(rules); + return true; + } + /// The route as stored, or null when there is none. /// /// Missing parts come through as empty values rather than making the route vanish, so that @@ -447,6 +509,9 @@ internal sealed class HostPayloadDocument /// public string[]? PinnedPaths { get; set; } + /// Last, and null rather than [], for the reasons gives. + public PortForwardDocument?[]? PortForwards { get; set; } + /// /// Last, and null for a host that is dialled, for the reason gives. One nested object /// rather than three loose properties, because the three are one decision — a server id without its @@ -465,6 +530,33 @@ internal sealed class SpelunkPayloadDocument public Guid? ApiKeyCredentialId { get; set; } } +/// One forward's serialised shape. Nullable throughout, because it models untrusted input. +/// +/// The kind is written as its number rather than its name, so renaming a member of +/// can never change what is on the wire. +/// +internal sealed class PortForwardDocument +{ + public Guid? Id { get; set; } + + public int? Kind { get; set; } + + public int? BindPort { get; set; } + + public string? TargetHost { get; set; } + + public int? TargetPort { get; set; } + + public static PortForwardDocument From(PortForwardRule rule) => new() + { + Id = rule.Id, + Kind = (int)rule.Kind, + BindPort = rule.BindPort, + TargetHost = rule.TargetHost, + TargetPort = rule.TargetPort, + }; +} + [JsonSourceGenerationOptions( PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase, DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, diff --git a/src/DodoSSH.Client.Domain/HostSecretMerge.cs b/src/DodoSSH.Client.Domain/HostSecretMerge.cs index 5397e01..e33d1db 100644 --- a/src/DodoSSH.Client.Domain/HostSecretMerge.cs +++ b/src/DodoSSH.Client.Domain/HostSecretMerge.cs @@ -99,6 +99,8 @@ public static class HostSecretMerge TagIds = MergeTags(ancestor.TagIds, local.TagIds, remote.TagIds, conflicts), PinnedPaths = MergePinnedPaths( ancestor.PinnedPaths, local.PinnedPaths, remote.PinnedPaths, conflicts), + PortForwards = MergePortForwards( + ancestor.PortForwards, local.PortForwards, remote.PortForwards, conflicts), RelayEnabled = Field( nameof(HostSecret.RelayEnabled), ancestor.RelayEnabled, @@ -354,6 +356,46 @@ public static class HostSecretMerge return PinnedPathList.Create(merge.Merged.Keys); } + /// + /// Merges the forwards per forward, so two people each adding one both keep theirs. + /// + /// + /// + /// Keyed by id and ordered the way is: existing forwards, then this + /// machine's additions, then the server's. + /// + /// + /// Unlike a tag or a pinned path, a conflict is reachable here. The value under a key is the whole + /// rule, so two machines changing one forward differently is a genuine clash. The server's version wins, + /// as every scalar's does, and the other is recorded in OpenSSH's notation so the user can read what was + /// lost. An edit on one side beats a removal on the other, which is the map merge's standing rule. + /// + /// + private static PortForwardList MergePortForwards( + PortForwardList ancestor, + PortForwardList local, + PortForwardList remote, + List conflicts) + { + var merge = ThreeWayMerge.Map( + ancestor.ToIdMap(), + local.ToIdMap(), + remote.ToIdMap(), + EqualityComparer.Default); + + foreach (var conflict in merge.Conflicts) + { + conflicts.Add(new HostFieldConflict( + $"{nameof(HostSecret.PortForwards)}[{conflict.Key}]", + conflict.DiscardedSide, + conflict.Kept?.Notation ?? "removed", + conflict.Discarded?.Notation ?? "removed", + conflict.DiscardedWasRemoval)); + } + + return PortForwardList.Create(merge.Merged.Values); + } + private static string FormatChain(JumpChain chain) => chain.Count == 0 ? "(none)" : string.Join(" → ", chain); } diff --git a/src/DodoSSH.Client.Domain/PortForwardList.cs b/src/DodoSSH.Client.Domain/PortForwardList.cs new file mode 100644 index 0000000..2a9cf77 --- /dev/null +++ b/src/DodoSSH.Client.Domain/PortForwardList.cs @@ -0,0 +1,140 @@ +using System.Collections; +using System.Diagnostics.CodeAnalysis; + +namespace DodoSSH.Client.Domain; + +/// +/// The port forwards saved on a host, in the order the user added them. +/// +/// +/// +/// A dedicated type rather than a list, for the reason gives: a plain list on a +/// record compares by reference, so every host would read as changed on every sync pass. +/// +/// +/// Ordered and unique by id. Order is how the session sidebar lists them. Uniqueness is by +/// rather than by value, keeping the first occurrence, because the id is +/// what the merge keys on — two entries with one id would be one forward the merge could not tell apart. +/// +/// +/// Invalid rules are not rejected at construction, for the reason every collection in this assembly gives: +/// they arrive from another client's payload, and a throw here would fail the whole sync pass. +/// catches them. +/// +/// +public sealed class PortForwardList : IReadOnlyList, IEquatable +{ + private readonly PortForwardRule[] rules; + private readonly int hash; + + private PortForwardList(PortForwardRule[] rules) + { + this.rules = rules; + hash = ComputeHash(rules); + } + + /// No forwards. + public static PortForwardList Empty { get; } = new([]); + + /// + public int Count => rules.Length; + + /// + public PortForwardRule this[int index] => rules[index]; + + /// Copies a sequence of rules, keeping order and the first rule with any given id. + public static PortForwardList Create(IEnumerable rules) + { + ArgumentNullException.ThrowIfNull(rules); + + var kept = new List(); + var seen = new HashSet(); + + foreach (var rule in rules) + { + if (rule is null) + { + throw new ArgumentException("A forward list cannot contain a null rule.", nameof(rules)); + } + + if (seen.Add(rule.Id)) + { + kept.Add(rule); + } + } + + return kept.Count == 0 ? Empty : new PortForwardList([.. kept]); + } + + /// + /// The list as a map from id to rule, in list order, which is the shape + /// takes. + /// + /// + /// Built in list order for the reason is: the merge unions keys + /// ancestor first, then local, then remote, so an ordered map is what keeps "existing forwards, then + /// additions" without the merge ordering anything itself. Unlike a pinned path, a value here can change + /// under its key, so a conflict is reachable: both machines editing the same forward differently. + /// + public IReadOnlyDictionary ToIdMap() + { + var map = new Dictionary(rules.Length); + + foreach (var rule in rules) + { + map[rule.Id] = rule; + } + + return map; + } + + /// + public bool Equals(PortForwardList? other) + { + if (ReferenceEquals(this, other)) + { + return true; + } + + return other is not null + && other.hash == hash + && rules.AsSpan().SequenceEqual(other.rules); + } + + /// + public override bool Equals(object? obj) => Equals(obj as PortForwardList); + + /// + public override int GetHashCode() => hash; + + /// + public IEnumerator GetEnumerator() => ((IEnumerable)rules).GetEnumerator(); + + /// + IEnumerator IEnumerable.GetEnumerator() => rules.GetEnumerator(); + + /// Contents equality, tolerating nulls on either side. + [SuppressMessage( + "Usage", + "CA2225:Operator overloads have named alternates", + Justification = "Equals(PortForwardList) is the named alternate.")] + public static bool operator ==(PortForwardList? left, PortForwardList? right) => + left is null ? right is null : left.Equals(right); + + /// Contents inequality. + public static bool operator !=(PortForwardList? left, PortForwardList? right) => !(left == right); + + private static int ComputeHash(PortForwardRule[] rules) + { + // Order-sensitive, because order is kept. + var accumulator = new HashCode(); + accumulator.Add(rules.Length); + + foreach (var rule in rules) + { + accumulator.Add(rule); + } + + return accumulator.ToHashCode(); + } +} diff --git a/src/DodoSSH.Client.Domain/PortForwardRule.cs b/src/DodoSSH.Client.Domain/PortForwardRule.cs new file mode 100644 index 0000000..436dd5c --- /dev/null +++ b/src/DodoSSH.Client.Domain/PortForwardRule.cs @@ -0,0 +1,166 @@ +using System.Diagnostics.CodeAnalysis; +using System.Globalization; + +namespace DodoSSH.Client.Domain; + +/// Which way a saved forward carries connections. +/// +/// Stored as its number, so the values are part of the payload format and must never be renumbered. The +/// SSH layer has its own enum with the same three members; the connect path maps one to the other, which +/// keeps this assembly free of any reference to it. +/// +public enum PortForwardKind +{ + /// A port on this machine, carried to a target the remote can reach. ssh -L. + Local = 0, + + /// A port on the remote, carried back to a target this machine can reach. ssh -R. + Remote = 1, + + /// A SOCKS proxy on this machine, whose connections leave from the remote. ssh -D. + Dynamic = 2, +} + +/// +/// A port forward saved on a host, started with every terminal opened to it. +/// +/// +/// +/// Part of the host's encrypted payload rather than an item of its own, which is what +/// RelayTicketRequest.PortForwardId has said since the contract was frozen. A forward means nothing +/// without the host it runs over, it is edited in the host's own editor, and a separate item kind would be a +/// table, a server kind, a cipher and a repository to buy one thing the host's per-id merge already gives: +/// two machines each adding a forward both keep theirs. SyncEntityType.PortForward stays reserved, +/// the way HostTag does for the same reason. +/// +/// +/// There is no bind address. Every listener binds loopback — see SshPortForwardSpec for why a +/// field another member of the keychain can fill in must not be able to say otherwise. +/// +/// +/// is what the merge keys on, so editing a forward's port on one machine and its target on +/// another is one forward changed twice — a conflict the merge records — rather than two forwards. +/// +/// +public sealed record PortForwardRule +{ + /// The longest a target host may be, which is the longest a DNS name can be. + public const int MaxTargetHostLength = 253; + + /// Identifies this forward within its host, for the merge. + public required Guid Id { get; init; } + + /// Which way it carries connections. + public required PortForwardKind Kind { get; init; } + + /// + /// The port to listen on: on this machine for a local or dynamic forward, on the remote for a remote one. + /// + public required int BindPort { get; init; } + + /// Where each connection goes, or null for a dynamic forward. + /// + /// Resolved by the remote for a local forward — so it may name a machine only the remote can see — and + /// by this machine for a remote one. + /// + public string? TargetHost { get; init; } + + /// The port on , or null for a dynamic forward. + public int? TargetPort { get; init; } + + /// + /// The forward in OpenSSH's own notation, which is what anybody who has used one will read fastest. + /// + /// + /// -L 5432:db.internal:5432, -R 8080:localhost:3000, -D 1080. No bind address, because + /// there is never one to show. + /// + public string Notation => Kind switch + { + PortForwardKind.Local => $"-L {Port(BindPort)}:{TargetHost}:{Port(TargetPort)}", + PortForwardKind.Remote => $"-R {Port(BindPort)}:{TargetHost}:{Port(TargetPort)}", + _ => $"-D {Port(BindPort)}", + }; + + /// Checks the fields that must hold before this can be stored. + /// + /// Deliberately silent about two forwards on one port. Two machines each adding one is exactly what the + /// merge produces, and a host that failed validation after a merge could not be written back at all; the + /// second listener fails when it starts instead, and says so. The editor refuses the duplicate up front. + /// + public bool TryValidate([NotNullWhen(false)] out string? reason) + { + if (Id == Guid.Empty) + { + reason = "A port forward cannot have an empty id."; + return false; + } + + if (!Enum.IsDefined(Kind)) + { + reason = $"'{(int)Kind}' is not a kind of port forward."; + return false; + } + + if (BindPort is < 1 or > 65535) + { + reason = $"A forward's listening port must be between 1 and 65535, not {BindPort}."; + return false; + } + + if (Kind is PortForwardKind.Dynamic) + { + if (TargetHost is not null || TargetPort is not null) + { + reason = "A dynamic forward has no target: each connection through it names its own."; + return false; + } + + reason = null; + return true; + } + + return TargetIsStorable(out reason); + } + + private static string Port(int? port) => port?.ToString(CultureInfo.InvariantCulture) ?? "?"; + + /// + /// Whitespace and control characters are refused as well as blanks: neither is part of any host name, + /// both are what a corrupted payload looks like, and this string is drawn on screen as it stands. + /// + private bool TargetIsStorable([NotNullWhen(false)] out string? reason) + { + if (string.IsNullOrWhiteSpace(TargetHost)) + { + reason = "A forward needs a host to forward to."; + return false; + } + + if (TargetHost.Length > MaxTargetHostLength) + { + reason = $"A forward's target cannot be longer than {MaxTargetHostLength} characters."; + return false; + } + + foreach (var ch in TargetHost) + { + if (char.IsControl(ch) || char.IsWhiteSpace(ch)) + { + reason = "A forward's target cannot contain spaces or control characters."; + return false; + } + } + + if (TargetPort is not (>= 1 and <= 65535)) + { + reason = TargetPort is { } port + ? $"A forward's target port must be between 1 and 65535, not {port}." + : "A forward needs a port to forward to."; + return false; + } + + reason = null; + return true; + } +} diff --git a/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs index 4690bd1..a67d7ff 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/MainWindowViewModel.cs @@ -1241,6 +1241,18 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp /// internal event EventHandler? TerminalSessionOpened; + /// + /// Raised when any tab's forward starts, stops or fails, so a head can re-count what is listening. + /// + /// + /// The phone's keep-alive notification is the one listener: it says how many forwards are listening + /// alongside how many shells are connected. On the UI thread, since every state change is. + /// + internal event EventHandler? ForwardsChanged; + + /// How many forwards are listening, across every tab. + internal int ListeningForwardCount => Tabs.Sum(tab => tab.ListeningForwardCount); + /// /// Raised when something this shell did belongs in the terminal the user is already looking at, so the /// view can put the keyboard back there. @@ -1487,6 +1499,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp internal bool IsTerminalShowing => IsTerminalSurface && !IsConnectSheetOpen + && !IsForwardsSheetOpen && !IsHostKeyDecisionShowing && SelectedTab is { HasSession: true }; @@ -2232,6 +2245,45 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp [RelayCommand] private void CloseConnectSheet() => IsConnectSheetOpen = false; + // ---- The phone's forwards sheet ---- + + /// + /// Whether the phone's list of the selected tab's forwards is open over the terminal. + /// + /// + /// + /// The phone's answer to the desktop sidebar's FORWARDS section, over the same rows — + /// — with the same state word, reason and switch. A sheet + /// rather than something always drawn because a phone gives a shell the whole screen, so it is raised + /// from a button in the terminal's bar, which draws only for a tab that has forwards and turns the danger + /// colour when one of them failed. + /// + /// + /// Shell state for the reason is: it collapses the renderer, which is a + /// native view nothing can be drawn over. Lowered with the connect sheet whenever the terminal surface is + /// left — see . + /// + /// + [ObservableProperty] + private bool isForwardsSheetOpen; + + /// Raises the forwards sheet over the terminal. + /// Gated as is, and on there being a tab to list. + [RelayCommand] + private void OpenForwardsSheet() + { + if (!IsTerminalSurface || SelectedTab is null) + { + return; + } + + IsForwardsSheetOpen = true; + } + + /// Lowers the forwards sheet, leaving the terminal where it was. + [RelayCommand] + private void CloseForwardsSheet() => IsForwardsSheetOpen = false; + // ---- Open terminals ---- /// @@ -3868,11 +3920,13 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp IdentityLabel = e.IdentityLabel, }; AdoptTab(adopted); + StartForwards(adopted, e); RefreshConnectedHosts(); return; } tab.Opened(e.SessionId); + StartForwards(tab, e); // From this moment, not from when the tab first appeared — connecting is not open, and the session // shell's elapsed timer is about a shell that is actually running. @@ -3894,6 +3948,54 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp TerminalSessionOpened?.Invoke(this, EventArgs.Empty); } + /// + /// Puts the host's saved forwards on a tab whose session has just opened, and starts every one. + /// + /// + /// + /// All at once and none awaited: each row reports its own outcome, and the terminal is already usable + /// while a remote forward waits for the server to answer. See for why + /// one failing stops nothing else. + /// + /// + /// The session id is captured into the start delegate rather than read off the tab when a row is + /// toggled later. A reconnect puts a new id on the same tab, and a row from the old session that reached + /// for the tab's current id would start its listener on a connection it was never part of. + /// + /// + /// ◆ On the phone too. This used to return early on Android, because that head had nowhere to show + /// what was listening. Its terminal bar now carries a FORWARDS button over the same rows the desktop's + /// sidebar lists — see — so both heads start the same forwards. + /// + /// + private void StartForwards(TerminalTabViewModel tab, TerminalSessionEventArgs e) + { + var sessionId = e.SessionId; + + tab.ReplaceForwards(e.Forwards.Select(rule => new PortForwardViewModel( + rule, + (spec, cancellationToken) => workspace.StartForwardAsync(sessionId, spec, cancellationToken), + post))); + + foreach (var forward in tab.Forwards) + { + // Never unsubscribed, and it need not be: the row holds the handler, not the other way round, so a + // row replaced by a reconnect is collected with it — after its last change, to Stopped, is heard. + forward.PropertyChanged += OnForwardChanged; + _ = forward.StartAsync(); + } + + ForwardsChanged?.Invoke(this, EventArgs.Empty); + } + + private void OnForwardChanged(object? sender, PropertyChangedEventArgs e) + { + if (e.PropertyName is nameof(PortForwardViewModel.State)) + { + ForwardsChanged?.Invoke(this, EventArgs.Empty); + } + } + /// /// Answers a connection that did not become a session. /// @@ -4694,6 +4796,17 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp vault.EditSelectedHostCommand.Execute(null); } + /// + /// The sidebar's FORWARDS affordance: opens the active tab's host for editing, where forwards are added. + /// + /// + /// The same route takes, because a forward is saved on the host for + /// the same reason a pin is. A forward added there starts with the next session to the host, not this + /// one — the editor says so — since this session's forwards were started from the host as it was. + /// + [RelayCommand] + private void AddForwardFromSidebar() => PinFolderFromSidebar(); + /// /// The SFTP tab row's click: makes one of the terminal's tabs the SFTP surface's browsed host. /// @@ -5169,6 +5282,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp if (value is not ShellSurface.Terminal) { IsConnectSheetOpen = false; + IsForwardsSheetOpen = false; } RaiseSurfaceState(); @@ -5353,6 +5467,9 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp /// partial void OnIsConnectSheetOpenChanged(bool value) => RaiseTerminalState(); + /// + partial void OnIsForwardsSheetOpenChanged(bool value) => RaiseTerminalState(); + /// /// The unlock card and the confirmation swap, so arming one has to hide the other — see /// . diff --git a/src/DodoSSH.Client.Shell/ViewModels/PortForwardViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/PortForwardViewModel.cs new file mode 100644 index 0000000..5d5942b --- /dev/null +++ b/src/DodoSSH.Client.Shell/ViewModels/PortForwardViewModel.cs @@ -0,0 +1,249 @@ +using System.Globalization; +using CommunityToolkit.Mvvm.ComponentModel; +using CommunityToolkit.Mvvm.Input; +using DodoSSH.Client.Domain; +using DodoSSH.Client.Ssh; + +namespace DodoSSH.Client.Shell.ViewModels; + +/// Where one of a tab's forwards has got to. +internal enum PortForwardState +{ + /// Not listening: switched off, or its session is over. + Stopped = 0, + + /// Asked for and not answered yet. A remote forward waits on the server. + Starting = 1, + + /// Listening. + Listening = 2, + + /// Could not be started; says why. + Failed = 3, +} + +/// +/// One of a host's saved forwards, as it runs on one terminal tab. +/// +/// +/// +/// Built fresh for each session the tab holds, and started with it: the rule is the host's, the listener is +/// this session's. A reconnect is a new session, so it gets new rows — see +/// — rather than trying to carry a listener across a +/// connection that no longer exists. +/// +/// +/// One failing does not stop the others, or the terminal. The commonest failure is a port already in +/// use — very often by a second tab to the same host, which is running the same forwards — and a shell that +/// would not open because of it would be the forward taking the session hostage. The row says why instead. +/// +/// +internal sealed partial class PortForwardViewModel : ObservableObject +{ + private readonly Func> start; + private readonly Action post; + + private ISshPortForward? forward; + private bool sessionOver; + + /// The detail line: why it failed, or what last went wrong through it. + [ObservableProperty] + private string? detail; + + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(IsListening))] + [NotifyPropertyChangedFor(nameof(IsFailed))] + [NotifyPropertyChangedFor(nameof(CanToggle))] + [NotifyPropertyChangedFor(nameof(StateText))] + [NotifyCanExecuteChangedFor(nameof(ToggleCommand))] + private PortForwardState state; + + /// The host's saved forward. + /// Starts a forward on this tab's session; see TerminalWorkspace.StartForwardAsync. + /// Runs an action on the UI thread, for reports SSH.NET raises on its own. + internal PortForwardViewModel( + PortForwardRule rule, + Func> start, + Action post) + { + Rule = rule; + this.start = start; + this.post = post; + } + + /// The saved forward this row runs. + internal PortForwardRule Rule { get; } + + /// The forward in OpenSSH's notation, e.g. -L 5432:db.internal:5432. + internal string Notation => Rule.Notation; + + /// What the forward connects to what, in words rather than flags. + /// + /// Which end each port is on is the thing people get backwards about -L and -R, so it is + /// written out: "this machine" and "the remote" rather than an arrow whose direction has to be learned. + /// + internal string Route => Rule.Kind switch + { + PortForwardKind.Local => + $"localhost:{Port(Rule.BindPort)} here → {Rule.TargetHost}:{Port(Rule.TargetPort)} from the remote", + PortForwardKind.Remote => + $"localhost:{Port(Rule.BindPort)} on the remote → {Rule.TargetHost}:{Port(Rule.TargetPort)} from here", + _ => $"SOCKS proxy on localhost:{Port(Rule.BindPort)}, out through the remote", + }; + + /// Whether it is listening now. + internal bool IsListening => State is PortForwardState.Listening; + + /// Whether it could not be started. + internal bool IsFailed => State is PortForwardState.Failed; + + /// + /// Whether the switch does anything: not while a start is in flight, and never once the session is over. + /// + internal bool CanToggle => !sessionOver && State is not PortForwardState.Starting; + + /// The state in the status bar's own lettering. + internal string StateText => State switch + { + PortForwardState.Starting => "STARTING", + PortForwardState.Listening => "LISTENING", + PortForwardState.Failed => "FAILED", + _ => "OFF", + }; + + /// Starts the forward, unless it is already running or its session has ended. + /// + /// Never throws. It is started without anyone waiting on it — every forward on a tab at once, as the + /// tab opens — so a failure has nowhere to go but this row. + /// + internal async Task StartAsync() + { + if (sessionOver || State is PortForwardState.Starting or PortForwardState.Listening) + { + return; + } + + State = PortForwardState.Starting; + Detail = null; + + ISshPortForward started; + + try + { + started = await start(ToSpec(Rule), CancellationToken.None).ConfigureAwait(true); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + // Unless the session ended while this was in flight, in which case that is the true story and + // SessionEnded has already told it. + if (!sessionOver) + { + Detail = exception.Message; + State = PortForwardState.Failed; + } + + return; + } + + if (sessionOver) + { + // The session ended while the server was answering. Its connection has gone and taken the + // listener with it; this only lets go of the handle. + await started.DisposeAsync().ConfigureAwait(true); + return; + } + + forward = started; + started.ErrorOccurred += OnForwardError; + State = PortForwardState.Listening; + } + + /// Stops the forward and leaves the row switched off. + internal async Task StopAsync() + { + await ReleaseAsync().ConfigureAwait(true); + + if (!sessionOver) + { + State = PortForwardState.Stopped; + Detail = null; + } + } + + /// + /// Records that the session this forward ran over has ended, which stopped the forward with it. + /// + /// + /// A failed row keeps its reason: that the session later ended does not make it any less true that the + /// port was taken. Anything else is off, and says it went with the session rather than being switched + /// off, which is the difference between "I did that" and "that happened". + /// + internal void SessionEnded() + { + if (sessionOver) + { + return; + } + + sessionOver = true; + _ = ReleaseAsync(); + + if (State is not PortForwardState.Failed) + { + State = PortForwardState.Stopped; + Detail = "Stopped with the session."; + } + + OnPropertyChanged(nameof(CanToggle)); + ToggleCommand.NotifyCanExecuteChanged(); + } + + /// The SSH layer's spec for a saved rule; the two enums share their numbering on purpose. + internal static SshPortForwardSpec ToSpec(PortForwardRule rule) => rule.Kind switch + { + PortForwardKind.Local => new SshPortForwardSpec( + SshPortForwardKind.Local, rule.BindPort, rule.TargetHost, rule.TargetPort ?? 0), + PortForwardKind.Remote => new SshPortForwardSpec( + SshPortForwardKind.Remote, rule.BindPort, rule.TargetHost, rule.TargetPort ?? 0), + _ => new SshPortForwardSpec(SshPortForwardKind.Dynamic, rule.BindPort), + }; + + /// A failed row's switch is a retry: the port may have been freed since. + [RelayCommand(CanExecute = nameof(CanToggle))] + private Task ToggleAsync() => IsListening ? StopAsync() : StartAsync(); + + private async Task ReleaseAsync() + { + if (forward is not { } running) + { + return; + } + + forward = null; + running.ErrorOccurred -= OnForwardError; + + try + { + await running.DisposeAsync().ConfigureAwait(true); + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + // Stopping a listener has no failure anybody could act on; the connection closing would have + // stopped it regardless. + } + } + + /// + /// A connection through the forward failed — usually the target refusing it. The listener is still up, + /// so the row stays LISTENING and says what happened underneath. + /// + private void OnForwardError(object? sender, SshPortForwardErrorEventArgs e) => post(() => + { + if (State is PortForwardState.Listening) + { + Detail = e.Message; + } + }); + + private static string Port(int? port) => port?.ToString(CultureInfo.InvariantCulture) ?? "?"; +} diff --git a/src/DodoSSH.Client.Shell/ViewModels/TerminalTabViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/TerminalTabViewModel.cs index b99c28e..120f4bc 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/TerminalTabViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/TerminalTabViewModel.cs @@ -1,3 +1,6 @@ +using System.Collections.ObjectModel; +using System.ComponentModel; +using System.Globalization; using CommunityToolkit.Mvvm.ComponentModel; using DodoSSH.Client.Ssh; @@ -326,6 +329,90 @@ internal sealed partial class TerminalTabViewModel : ObservableObject /// How many steps there are, for the card's track. internal int StepCount => Steps.Count; + /// + /// The host's saved forwards, as they run over this tab's current session. + /// + /// + /// Replaced whole each time a session opens here — see — and emptied of + /// meaning, though not of rows, when it ends: a row that says it stopped with the session is more use + /// than one that silently disappeared. + /// + internal ObservableCollection Forwards { get; } = []; + + /// How many of are listening now. + internal int ListeningForwardCount => Forwards.Count(forward => forward.IsListening); + + /// Whether any of is listening now. + internal bool HasListeningForward => ListeningForwardCount > 0; + + /// Whether this tab carries any forwards at all, running or not. + /// The phone's terminal bar draws its FORWARDS button only for a tab that has some. + internal bool HasForwards => Forwards.Count > 0; + + /// Whether any of could not be started. + /// + /// The phone's list is a sheet, not a sidebar that is always drawn, so a refusal there would be invisible + /// until somebody happened to open it. This is what lets the button that opens it say so first. + /// + internal bool HasFailedForward => Forwards.Any(forward => forward.IsFailed); + + /// + /// The status bar's forward count, or null when nothing is listening and there is nothing to say. + /// + internal string? ForwardsSummary => ListeningForwardCount switch + { + 0 => null, + 1 => "⇄ 1 FORWARD", + var count => string.Create(CultureInfo.InvariantCulture, $"⇄ {count} FORWARDS"), + }; + + /// Puts a new session's forwards on this tab, ending whatever the last session left. + internal void ReplaceForwards(IEnumerable forwards) + { + EndForwards(); + + foreach (var forward in Forwards) + { + forward.PropertyChanged -= OnForwardChanged; + } + + Forwards.Clear(); + + foreach (var forward in forwards) + { + forward.PropertyChanged += OnForwardChanged; + Forwards.Add(forward); + } + + RaiseForwardCount(); + } + + /// Tells every forward its session is over; SSH.NET has already stopped the listeners. + private void EndForwards() + { + foreach (var forward in Forwards) + { + forward.SessionEnded(); + } + } + + private void OnForwardChanged(object? sender, PropertyChangedEventArgs e) + { + if (e.PropertyName is nameof(PortForwardViewModel.State)) + { + RaiseForwardCount(); + } + } + + private void RaiseForwardCount() + { + OnPropertyChanged(nameof(ListeningForwardCount)); + OnPropertyChanged(nameof(ForwardsSummary)); + OnPropertyChanged(nameof(HasListeningForward)); + OnPropertyChanged(nameof(HasForwards)); + OnPropertyChanged(nameof(HasFailedForward)); + } + /// [ObservableProperty] private TerminalTabState state; @@ -603,6 +690,8 @@ internal sealed partial class TerminalTabViewModel : ObservableObject EndDescription = description; IsLive = false; + EndForwards(); + OnPropertyChanged(nameof(EndedDetail)); OnPropertyChanged(nameof(EndedSummary)); OnPropertyChanged(nameof(EndDescription)); @@ -638,6 +727,9 @@ internal sealed partial class TerminalTabViewModel : ObservableObject EndDescription = null; IsLive = false; + // The old session is being closed for its pane's sake, and its forwards go with its connection. + EndForwards(); + // Cleared with the session they were facts about. Keeping them would put the previous connection's // cipher in the status bar beside a handshake that has not negotiated one yet. StartedAt = null; diff --git a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs index 041b9e5..a0bc153 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs @@ -594,6 +594,10 @@ internal sealed partial class HostRowViewModel( /// Whether the card draws a pin-count badge at all. internal bool HasPins => PinCount > 0; + /// Whether this host carries any port forwards, for the detail pane's FORWARDS card. + /// Read off the secret for the reason is. + internal bool HasForwards => host.Secret.PortForwards.Count > 0; + /// /// The two letters a card's monogram avatar draws. /// @@ -956,8 +960,17 @@ internal sealed class TerminalSessionEventArgs( string cipher, string hostKeyAlgorithm, string? identityLabel, - Guid? hostId) : EventArgs + Guid? hostId, + PortForwardList? forwards = null) : EventArgs { + /// + /// The forwards to start on this session: the host's saved ones, or none for a typed-in address. + /// + /// + /// Carried here because the shell has no vault to read them from, for the reason the label is. + /// + internal PortForwardList Forwards { get; } = forwards ?? PortForwardList.Empty; + /// Which attempt this session came out of. /// internal Guid AttemptId { get; } = attemptId; @@ -3723,6 +3736,196 @@ internal sealed partial class VaultViewModel( EditorNewPin = string.Empty; } + /// + /// The port forwards on the host being edited, staged the way is. + /// + /// + /// Populated when the editor opens, changed by and + /// , read back by on Save. A rule keeps its id + /// through the editor, which is what lets the merge tell an edited forward from a new one. + /// + internal ObservableCollection EditorPortForwards { get; } = []; + + /// The add row's kind, as the index of : local, remote, dynamic. + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(EditorNewForwardHasTarget))] + [NotifyPropertyChangedFor(nameof(EditorNewForwardBindHint))] + [NotifyPropertyChangedFor(nameof(EditorNewForwardKindHint))] + private int editorNewForwardKind; + + /// The add row's listening port, as typed. + [ObservableProperty] + private string editorNewForwardBindPort = string.Empty; + + /// The add row's target, as host:port. + [ObservableProperty] + private string editorNewForwardTarget = string.Empty; + + /// Whether the add row's kind has a target at all; a dynamic forward does not. + internal bool EditorNewForwardHasTarget => (PortForwardKind)EditorNewForwardKind is not PortForwardKind.Dynamic; + + /// The add row's listening-port placeholder: short, because the box is narrow. + internal string EditorNewForwardBindHint => (PortForwardKind)EditorNewForwardKind switch + { + PortForwardKind.Remote => "remote port", + PortForwardKind.Dynamic => "SOCKS port", + _ => "local port", + }; + + /// + /// What the add row's kind does, in a sentence naming which end each port is on — the thing people get + /// backwards about -L and -R. + /// + internal string EditorNewForwardKindHint => (PortForwardKind)EditorNewForwardKind switch + { + PortForwardKind.Remote => + "A port on the remote, carried back to a host:port as this machine sees it.", + PortForwardKind.Dynamic => + "A SOCKS proxy on this machine. Each connection through it leaves from the remote.", + _ => "A port on this machine, carried to a host:port as the remote sees it.", + }; + + /// + /// Adds a forward to the host being edited, from the editor's add row. + /// + /// + /// + /// Validated here rather than at Save, for the reason is: a refusal at the + /// box that caused it costs nothing else on the form. The rule's own + /// is the check, so this cannot drift from what the codec + /// accepts. + /// + /// + /// A second listener on a port this host already listens on is refused here, though the domain + /// allows it — it has to, because two machines each adding one is what a merge produces. Here it is one + /// person, and the second would only ever fail to start. + /// + /// + [RelayCommand] + private void AddEditorForward() + { + var kind = (PortForwardKind)EditorNewForwardKind; + + if (!int.TryParse(EditorNewForwardBindPort.Trim(), NumberStyles.None, CultureInfo.InvariantCulture, out var bindPort)) + { + Status = "A forward needs a listening port, as a number."; + return; + } + + string? targetHost = null; + int? targetPort = null; + + if (kind is not PortForwardKind.Dynamic) + { + if (!TryParseForwardTarget(EditorNewForwardTarget, out targetHost, out targetPort)) + { + Status = "A forward's target is host:port, for example db.internal:5432."; + return; + } + } + + var rule = new PortForwardRule + { + Id = Guid.CreateVersion7(), + Kind = kind, + BindPort = bindPort, + TargetHost = targetHost, + TargetPort = targetPort, + }; + + if (!rule.TryValidate(out var reason)) + { + Status = reason; + return; + } + + if (EditorPortForwards.Count >= HostSecret.MaxPortForwards) + { + Status = $"A host cannot carry more than {HostSecret.MaxPortForwards} port forwards."; + return; + } + + // Local and dynamic forwards both listen on this machine, so they share its ports; a remote forward + // listens on the remote, which is a different set. + var listensHere = kind is not PortForwardKind.Remote; + + if (EditorPortForwards.Any(existing => + existing.BindPort == bindPort && existing.Kind is not PortForwardKind.Remote == listensHere)) + { + Status = listensHere + ? $"This host already has a forward listening on port {bindPort} here." + : $"This host already has a forward listening on port {bindPort} on the remote."; + return; + } + + EditorPortForwards.Add(rule); + EditorNewForwardBindPort = string.Empty; + EditorNewForwardTarget = string.Empty; + Status = string.Empty; + } + + /// Removes a forward from the host being edited. + [RelayCommand] + private void RemoveEditorForward(PortForwardRule? rule) + { + if (rule is not null) + { + EditorPortForwards.Remove(rule); + } + } + + /// Stages an existing host's forwards for editing. See . + private void LoadEditorPortForwards(PortForwardList forwards) + { + EditorPortForwards.Clear(); + + foreach (var forward in forwards) + { + EditorPortForwards.Add(forward); + } + + EditorNewForwardKind = (int)PortForwardKind.Local; + EditorNewForwardBindPort = string.Empty; + EditorNewForwardTarget = string.Empty; + } + + /// + /// Reads host:port, with an IPv6 literal in brackets as OpenSSH writes one: [::1]:5432. + /// + /// + /// Split at the last colon, so an unbracketed IPv6 address fails to parse as a port rather than being + /// cut somewhere in its middle. The host is checked by the rule, not here. + /// + internal static bool TryParseForwardTarget(string text, out string? host, out int? port) + { + host = null; + port = null; + + var trimmed = text.Trim(); + var colon = trimmed.LastIndexOf(':'); + + if (colon <= 0 + || !int.TryParse(trimmed.AsSpan(colon + 1), NumberStyles.None, CultureInfo.InvariantCulture, out var parsed)) + { + return false; + } + + var name = trimmed[..colon]; + + if (name.StartsWith('[') && name.EndsWith(']')) + { + name = name[1..^1]; + } + else if (name.Contains(':', StringComparison.Ordinal)) + { + return false; + } + + host = name; + port = parsed; + return true; + } + /// The item being edited, or null when creating. private Guid? editingEntityId; @@ -7552,6 +7755,7 @@ internal sealed partial class VaultViewModel( BuildTagChoices(); EditorPinnedPaths.Clear(); EditorNewPin = string.Empty; + LoadEditorPortForwards(PortForwardList.Empty); // Closed rather than carried over, and it holds a password — see EditorNewCredentialPassword. ClearEditorCredentialForm(); @@ -7646,6 +7850,7 @@ internal sealed partial class VaultViewModel( ClearEditorCredentialForm(); LoadEditorPinnedPaths(row.Host.PinnedPaths); + LoadEditorPortForwards(row.Host.PortForwards); BuildEditorVaultChoices(editingHostVaultId); BuildGroupChoices(row.Host.GroupId); @@ -11817,7 +12022,8 @@ internal sealed partial class VaultViewModel( facts?.Cipher ?? string.Empty, facts?.HostKeyAlgorithm ?? string.Empty, authentication.IdentityLabel, - target.HostId)); + target.HostId, + target.Row?.Host.PortForwards)); // Last, and after the tab exists: keeping the password is a favour, and the session the user asked // for must not wait on a vault write to appear. @@ -12381,6 +12587,9 @@ internal sealed partial class VaultViewModel( // than twice. PinnedPaths = PinnedPathList.Create(EditorPinnedPaths), + // The staged list, in the order it was built; each rule keeps the id it arrived with. + PortForwards = PortForwardList.Create(EditorPortForwards), + // Both read off the one picker, including the id of something that has gone missing. Reading them // from the picker rather than carrying the originals through is what lets a binding be removed at // all, and preserving a missing id is what stops an unrelated edit removing one by accident. One diff --git a/src/DodoSSH.Client.Ssh/SshConnection.cs b/src/DodoSSH.Client.Ssh/SshConnection.cs index e136316..a85aec9 100644 --- a/src/DodoSSH.Client.Ssh/SshConnection.cs +++ b/src/DodoSSH.Client.Ssh/SshConnection.cs @@ -190,6 +190,15 @@ public interface ISshConnection : IAsyncDisposable /// Opens an interactive shell with a pseudo-terminal. Task OpenShellAsync(TerminalSize size, CancellationToken cancellationToken); + + /// Starts a port forward over this connection. + /// + /// Runs beside the shell on the same transport, as ssh -L does, and stops when the connection + /// does. Any number may be started; each is its own listener and fails on its own. + /// + /// The spec is not valid; see . + /// The forward could not be started, with a message saying why. + Task StartForwardAsync(SshPortForwardSpec spec, CancellationToken cancellationToken); } /// diff --git a/src/DodoSSH.Client.Ssh/SshNetConnectionFactory.cs b/src/DodoSSH.Client.Ssh/SshNetConnectionFactory.cs index 2df9c7d..eac59f8 100644 --- a/src/DodoSSH.Client.Ssh/SshNetConnectionFactory.cs +++ b/src/DodoSSH.Client.Ssh/SshNetConnectionFactory.cs @@ -417,6 +417,14 @@ internal sealed class SshNetConnection( return Task.FromResult(new SshNetShellSession(shell, client)); } + /// + public Task StartForwardAsync(SshPortForwardSpec spec, CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(spec); + + return SshNetPortForward.StartAsync(client, spec, cancellationToken); + } + /// public async ValueTask DisposeAsync() { diff --git a/src/DodoSSH.Client.Ssh/SshNetPortForward.cs b/src/DodoSSH.Client.Ssh/SshNetPortForward.cs new file mode 100644 index 0000000..94d17c9 --- /dev/null +++ b/src/DodoSSH.Client.Ssh/SshNetPortForward.cs @@ -0,0 +1,223 @@ +using System.Globalization; +using System.Net; +using System.Net.Sockets; +using Renci.SshNet; +using Renci.SshNet.Common; + +namespace DodoSSH.Client.Ssh; + +/// A forward over an SSH.NET client. +/// +/// +/// One class for all three kinds, because SSH.NET's three share a base with the only members this needs — +/// Start, Stop, IsStarted and the Exception event — and differ only in how they +/// are built and where their bound port is read from. +/// +/// +/// ◆ Started and stopped off the calling thread. A remote forward's Start sends +/// tcpip-forward and blocks until the server answers, and every kind's Stop waits for its open +/// channels to drain. Either would otherwise hold the UI thread for a round trip or longer. +/// +/// +internal sealed class SshNetPortForward : ISshPortForward +{ + /// Where every listener on this machine binds. See . + private const string LoopbackAddress = "127.0.0.1"; + + private readonly SshClient client; + private readonly ForwardedPort port; + + private string? lastError; + private int disposed; + + private SshNetPortForward(SshClient client, ForwardedPort port, SshPortForwardSpec spec) + { + this.client = client; + this.port = port; + Spec = spec; + + port.Exception += OnException; + } + + /// + public event EventHandler? ErrorOccurred; + + /// + public SshPortForwardSpec Spec { get; } + + /// + public int BoundPort => port switch + { + ForwardedPortLocal local => (int)local.BoundPort, + ForwardedPortDynamic dynamic => (int)dynamic.BoundPort, + ForwardedPortRemote remote => (int)remote.BoundPort, + _ => Spec.BindPort, + }; + + /// + /// + /// Read off the port rather than the client: SSH.NET stops every forward on a session that ends, and + /// SshClient.IsConnected throws rather than answering once the connection has been disposed. + /// + public bool IsListening => Volatile.Read(ref disposed) == 0 && port.IsStarted; + + /// + public string? LastError => Volatile.Read(ref lastError); + + /// Starts a forward on a connected client. + /// The spec is not valid. + /// The forward could not be started. + internal static async Task StartAsync( + SshClient client, + SshPortForwardSpec spec, + CancellationToken cancellationToken) + { + if (!spec.TryValidate(out var reason)) + { + throw new ArgumentException(reason, nameof(spec)); + } + + var listener = await CreateAsync(spec, cancellationToken).ConfigureAwait(false); + var forward = new SshNetPortForward(client, listener, spec); + + try + { + client.AddForwardedPort(listener); + + await Task.Run(listener.Start, cancellationToken).ConfigureAwait(false); + } + catch (Exception exception) when (exception is not OperationCanceledException) + { + await forward.DisposeAsync().ConfigureAwait(false); + + throw new SshPortForwardException(Explain(spec, exception), exception); + } + catch + { + await forward.DisposeAsync().ConfigureAwait(false); + throw; + } + + return forward; + } + + /// + /// + /// Safe on a client that has already gone: SSH.NET stops its forwards when the session ends, and what + /// is left to do then is let go of the port object, which cannot fail in a way anybody could act on. + /// + public async ValueTask DisposeAsync() + { + if (Interlocked.Exchange(ref disposed, 1) == 1) + { + return; + } + + port.Exception -= OnException; + + await Task.Run(() => + { + try + { + client.RemoveForwardedPort(port); + } + catch (Exception exception) when (exception is ObjectDisposedException or SshException + or SocketException or InvalidOperationException) + { + // The client or its session is already gone, and took the listener with it. + } + + port.Dispose(); + }).ConfigureAwait(false); + } + + /// + /// + /// A remote forward's target is resolved here, asynchronously, rather than by SSH.NET. Its string + /// constructor resolves both addresses synchronously and takes whichever comes first, so + /// localhost could become ::1 — which a service listening on 127.0.0.1 alone + /// refuses — and the bound address could be sent to the remote as an IPv6 literal it has no reason to + /// expect. IPv4 is preferred for the target for the first reason, and the bound address is the IPv4 + /// loopback literal for the second. + /// + /// + /// A local forward's target is not resolved at all. It travels to the remote as a name in the + /// direct-tcpip request, which is what lets it name a machine only the remote can see. + /// + /// + private static async Task CreateAsync(SshPortForwardSpec spec, CancellationToken cancellationToken) + { + switch (spec.Kind) + { + case SshPortForwardKind.Local: + return new ForwardedPortLocal( + LoopbackAddress, (uint)spec.BindPort, spec.TargetHost!, (uint)spec.TargetPort); + + case SshPortForwardKind.Dynamic: + return new ForwardedPortDynamic(LoopbackAddress, (uint)spec.BindPort); + + default: + var target = await ResolveAsync(spec.TargetHost!, cancellationToken).ConfigureAwait(false); + + return new ForwardedPortRemote(IPAddress.Loopback, (uint)spec.BindPort, target, (uint)spec.TargetPort); + } + } + + private static async Task ResolveAsync(string host, CancellationToken cancellationToken) + { + if (IPAddress.TryParse(host, out var literal)) + { + return literal; + } + + IPAddress[] addresses; + + try + { + addresses = await Dns.GetHostAddressesAsync(host, cancellationToken).ConfigureAwait(false); + } + catch (SocketException exception) + { + throw new SshPortForwardException($"'{host}' could not be resolved on this machine.", exception); + } + + return addresses.FirstOrDefault(address => address.AddressFamily is AddressFamily.InterNetwork) + ?? addresses.FirstOrDefault() + ?? throw new SshPortForwardException($"'{host}' could not be resolved on this machine."); + } + + /// Says which port could not be had, and where, rather than repeating a socket error code. + private static string Explain(SshPortForwardSpec spec, Exception exception) + { + var bind = spec.BindPort.ToString(CultureInfo.InvariantCulture); + + return exception switch + { + SshPortForwardException own => own.Message, + + SocketException { SocketErrorCode: SocketError.AddressAlreadyInUse } => + $"Port {bind} on this machine is already in use.", + + SocketException { SocketErrorCode: SocketError.AccessDenied } => + $"This machine would not let DodoSSH listen on port {bind}.", + + // What SSH.NET throws when the server answers tcpip-forward with a refusal. The server gives no + // reason, so neither can this — but the likely ones are worth naming. + SshException when spec.Kind is SshPortForwardKind.Remote => + $"The remote would not listen on port {bind}. It may be in use there, below 1024, or " + + "forbidden by the server's AllowTcpForwarding setting.", + + _ => exception.Message, + }; + } + + /// + /// Raised by SSH.NET on its own threads for a connection through the forward that failed — most often a + /// target that refused it. The listener stays up; see . + /// + private void OnException(object? sender, ExceptionEventArgs e) + { + Volatile.Write(ref lastError, e.Exception.Message); + ErrorOccurred?.Invoke(this, new SshPortForwardErrorEventArgs(e.Exception.Message)); + } +} diff --git a/src/DodoSSH.Client.Ssh/SshPortForward.cs b/src/DodoSSH.Client.Ssh/SshPortForward.cs new file mode 100644 index 0000000..c35fe6a --- /dev/null +++ b/src/DodoSSH.Client.Ssh/SshPortForward.cs @@ -0,0 +1,164 @@ +using System.Diagnostics.CodeAnalysis; + +namespace DodoSSH.Client.Ssh; + +/// Which way a forward carries connections. +/// The three OpenSSH has, named for the flag each one is: -L, -R and -D. +public enum SshPortForwardKind +{ + /// + /// A port on this machine, carried to a host and port as the remote sees them. ssh -L. + /// + Local = 0, + + /// + /// A port on the remote, carried back to a host and port as this machine sees them. ssh -R. + /// + Remote = 1, + + /// + /// A SOCKS proxy on this machine, whose every connection leaves from the remote. ssh -D. + /// + Dynamic = 2, +} + +/// One forward to run over a connection. +/// Which way it carries connections. +/// +/// The port to listen on: on this machine for and +/// , on the remote for . Zero +/// asks for an ephemeral port, which only a listener on this machine can report back — see +/// . +/// +/// +/// Where each connection goes: resolved by the remote for a local forward and by this machine for a remote +/// one. Null for a dynamic forward, whose target is whatever each SOCKS client asks for. +/// +/// The port on ; zero for a dynamic forward. +/// +/// +/// ◆ There is no bind address, and that is the point of the type. Every listener this client opens +/// is on loopback, for the reason gives: a forward bound to any other +/// interface hands the remote's network to whoever else can reach this machine, for as long as the session +/// is up. ssh -L 0.0.0.0:… is a flag somebody types knowing what it does; a field in a synced host +/// that another member of the keychain filled in is not. Leaving the address out makes that +/// unrepresentable rather than merely discouraged. +/// +/// +/// A remote forward asks for the remote's loopback too. Whether the remote honours that is its +/// GatewayPorts setting and not this client's to decide, which is the same arrangement OpenSSH has. +/// +/// +public sealed record SshPortForwardSpec( + SshPortForwardKind Kind, + int BindPort, + string? TargetHost = null, + int TargetPort = 0) +{ + /// Checks the spec can be started as it stands. + public bool TryValidate([NotNullWhen(false)] out string? reason) + { + if (!Enum.IsDefined(Kind)) + { + reason = $"'{Kind}' is not a kind of forward."; + return false; + } + + // A remote listener's port is chosen by the remote, and SSH.NET never reads the one it chose back, + // so an ephemeral remote forward would be listening somewhere nobody could name. + var lowest = Kind is SshPortForwardKind.Remote ? 1 : 0; + + if (BindPort < lowest || BindPort > 65535) + { + reason = $"The listening port must be between {lowest} and 65535, not {BindPort}."; + return false; + } + + if (Kind is SshPortForwardKind.Dynamic) + { + if (TargetHost is not null || TargetPort != 0) + { + reason = "A dynamic forward has no target: each connection through it names its own."; + return false; + } + + reason = null; + return true; + } + + if (string.IsNullOrWhiteSpace(TargetHost)) + { + reason = "A forward needs a host to forward to."; + return false; + } + + if (TargetPort is < 1 or > 65535) + { + reason = $"The target port must be between 1 and 65535, not {TargetPort}."; + return false; + } + + reason = null; + return true; + } +} + +/// Why a forward is no longer carrying a connection it accepted. +/// What went wrong, as SSH.NET put it. +public sealed class SshPortForwardErrorEventArgs(string message) : EventArgs +{ + /// What went wrong. + public string Message { get; } = message; +} + +/// A forward that is listening, and stops when disposed. +/// +/// +/// Owned by whoever started it, and also stopped by the connection it runs over: a connection that closes +/// takes every forward on it down with it, so disposing one afterwards is safe and does nothing. +/// +/// +/// ◆ No byte counts. SSH.NET copies a forwarded channel's bytes inside its own channel classes and +/// exposes no count of them, so a figure here would be an estimate presented as a measurement. +/// +/// +public interface ISshPortForward : IAsyncDisposable +{ + /// What was asked for. + SshPortForwardSpec Spec { get; } + + /// + /// The port actually listened on, which differs from only when + /// that asked for an ephemeral one. + /// + int BoundPort { get; } + + /// Whether the listener is still up. + /// + /// False once disposed, and once the connection under it has gone — SSH.NET stops every forward on a + /// session that ends. + /// + bool IsListening { get; } + + /// + /// The most recent failure to carry a connection, or null if there has not been one. + /// + /// + /// A failure here does not stop the listener. The usual cause is the target refusing one connection — + /// a database that is down, a port nothing is on — and the next connection may well succeed. + /// + string? LastError { get; } + + /// + /// Raised when a connection through this forward fails, on whichever thread SSH.NET noticed it. + /// + event EventHandler? ErrorOccurred; +} + +/// A forward that could not be started, with a sentence saying why. +/// +/// What SSH.NET throws for the two commonest refusals is a socket error code and "failed to start", neither +/// of which names the port. The message here does, and the original is kept as the inner exception. +/// +public sealed class SshPortForwardException(string message, Exception? innerException = null) + : Exception(message, innerException); diff --git a/src/DodoSSH.Client.Sync/ItemKinds.cs b/src/DodoSSH.Client.Sync/ItemKinds.cs index 7737501..9400082 100644 --- a/src/DodoSSH.Client.Sync/ItemKinds.cs +++ b/src/DodoSSH.Client.Sync/ItemKinds.cs @@ -274,6 +274,8 @@ internal sealed class HostKind : IItemKind Note(changed, "Password prompt", before.AsksForPassword, after.AsksForPassword); Note(changed, "Group", before.GroupId, after.GroupId); Note(changed, "Tags", before.TagIds, after.TagIds); + Note(changed, "Port forwards", before.PortForwards, after.PortForwards); + Note(changed, "Pinned paths", before.PinnedPaths, after.PinnedPaths); return changed; } diff --git a/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs b/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs index ff48e12..551956a 100644 --- a/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs +++ b/src/DodoSSH.Client.Terminal/TerminalWorkspace.cs @@ -550,6 +550,51 @@ public sealed class TerminalWorkspace : IAsyncDisposable return sessionId; } + /// + /// Starts a port forward over one terminal's connection. + /// + /// The terminal whose connection the forward runs over. + /// What to forward. + /// Abandons the start. + /// The forward, which the caller may dispose to stop it sooner. + /// + /// + /// The forward lives as long as the session, and this object does not keep a list of them. SSH.NET + /// stops every forward on a connection when the connection is disposed, which is what both + /// and the release of a session that ended on its own already do — so a + /// second record here would be a copy of a fact the library already enforces, and the one that could + /// disagree with it. Disposing a handle after its session has gone is safe. + /// + /// + /// Refused for a session that is not running, for the reason refuses one: a tab + /// whose shell ended is still on screen, and a forward started on its connection would be a listener + /// over a transport that is already being torn down. + /// + /// + /// The session is not running. + /// The forward could not be started. + public Task StartForwardAsync( + uint sessionId, + SshPortForwardSpec spec, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(spec); + + ISshConnection connection; + + lock (sessionGate) + { + if (!sessions.TryGetValue(sessionId, out var session) || session.Run.IsCompleted) + { + throw new InvalidOperationException("That terminal is no longer connected."); + } + + connection = session.Connection; + } + + return connection.StartForwardAsync(spec, cancellationToken); + } + /// /// Shows one terminal's pane and hides the others. /// diff --git a/tests/DodoSSH.Client.App.Layout.Tests/ScreenLayoutTests.cs b/tests/DodoSSH.Client.App.Layout.Tests/ScreenLayoutTests.cs index c30cc5c..777976a 100644 --- a/tests/DodoSSH.Client.App.Layout.Tests/ScreenLayoutTests.cs +++ b/tests/DodoSSH.Client.App.Layout.Tests/ScreenLayoutTests.cs @@ -187,6 +187,13 @@ public sealed class ScreenLayoutTests : IAsyncLifetime vault.EditorNewPin = "/var/www/a-fairly-long-application-directory-name"; vault.AddEditorPinCommand.Execute(null); + // And a forward, for the same reason: PORT FORWARDS draws its rows in the same mono face, and its + // add row is three controls abreast in a column 304 pixels wide. + vault.EditorNewForwardBindPort = "15432"; + vault.EditorNewForwardTarget = "customer-a-production-database.internal:5432"; + vault.AddEditorForwardCommand.Execute(null); + vault.EditorPortForwards.ShouldHaveSingleItem(vault.Status); + await MeasureDrawerAsync(faults => faults.ShouldBeEmpty()); } @@ -1013,9 +1020,10 @@ public sealed class ScreenLayoutTests : IAsyncLifetime } /// - /// The FIELDS column is the one that grows: it is a list of names, and a host has thirteen of them — - /// eleven until inheritance added "Password prompt" and tags added "Tags". Measured with an edit that - /// touched several, because one field name fits anywhere. The count is stated rather than derived, so + /// The FIELDS column is the one that grows: it is a list of names, and a host has fifteen of them — + /// eleven until inheritance added "Password prompt", tags added "Tags", forwards added "Port forwards" + /// and pinning added "Pinned paths". Measured with an edit that touched several, because one field + /// name fits anywhere. The count is stated rather than derived, so /// it has to be recounted against HostKind.Changes whenever a field is added; the literal in /// SeedLogsAsync is the thing that actually keeps the column measured at its worst. /// @@ -2244,7 +2252,7 @@ public sealed class ScreenLayoutTests : IAsyncLifetime ItemId = Guid.CreateVersion7(), ItemLabel = "customer-a-production-database", Operation = ActivityOperation.Updated, - ChangedFields = "Hostname, Port, Username, Options, Password prompt, Group, Tags", + ChangedFields = "Hostname, Port, Username, Options, Password prompt, Group, Tags, Port forwards, Pinned paths", At = new DateTimeOffset(2026, 7, 30, 9, 15, 0, TimeSpan.Zero), DeviceName = "jaap-jan-workstation", }, diff --git a/tests/DodoSSH.Client.App.Layout.Tests/SessionSidebarTests.cs b/tests/DodoSSH.Client.App.Layout.Tests/SessionSidebarTests.cs index 8412435..6e85155 100644 --- a/tests/DodoSSH.Client.App.Layout.Tests/SessionSidebarTests.cs +++ b/tests/DodoSSH.Client.App.Layout.Tests/SessionSidebarTests.cs @@ -4,6 +4,7 @@ using Avalonia.Headless; using Avalonia.Input; using Avalonia.VisualTree; using DodoSSH.Client.App.Views; +using DodoSSH.Client.Domain; using DodoSSH.Client.Session; using DodoSSH.Client.Shell.ViewModels; using DodoSSH.Client.Ssh; @@ -185,6 +186,73 @@ public sealed class SessionSidebarTests : IAsyncLifetime }); } + /// + /// FORWARDS at its widest: a local forward to a long name, listening, and one that could not start with + /// its reason wrapped under it. The state word sits at the end of the row beside a trimming notation, + /// which is the same shape as the address and its close button above — the word must not be pushed out. + /// + [Fact] + public async Task TheForwardsList_FitsWithALongTargetAndAFailureReason() + { + await OnTheSidebarAsync( + (_, window) => LayoutHarness.Unreachable(window).ShouldBeEmpty(), + tab => + { + var listening = Forward(15432, "customer-a-production-database.eu-west-1.internal", refusal: null); + var refused = Forward( + 8080, "web.internal", "The remote would not listen on port 8080. It may be in use there, below 1024, or forbidden by the server's AllowTcpForwarding setting."); + + tab.ReplaceForwards([listening, refused]); + + // Both starts complete synchronously against these stand-ins, so neither needs awaiting on a + // harness whose body is synchronous. + _ = listening.StartAsync(); + _ = refused.StartAsync(); + + listening.IsListening.ShouldBeTrue(); + refused.IsFailed.ShouldBeTrue(); + }); + } + + private static PortForwardViewModel Forward(int port, string target, string? refusal) => + new( + new PortForwardRule + { + Id = Guid.CreateVersion7(), + Kind = PortForwardKind.Local, + BindPort = port, + TargetHost = target, + TargetPort = port, + }, + (spec, _) => refusal is null + ? Task.FromResult(Substitute.For()) + : Task.FromException(new SshPortForwardException(refusal)), + action => action()); + + private Task OnTheSidebarAsync(Action body, Action arrange) => + LayoutHarness.OnTheUiThreadAsync( + () => + { + shell.Tabs.Add(new TerminalTabViewModel(1, "db-primary", LongAddress)); + shell.SelectTabCommand.Execute(shell.Tabs[0]); + + arrange(shell.Tabs[0]); + + var sidebar = new SessionSidebar { DataContext = shell, ShowsSnips = true, ShowsForwards = true }; + var window = LayoutHarness.HostAtMinimumSize( + sidebar, LayoutHarness.MinimumWidth, LayoutHarness.MinimumHeight); + + try + { + body(sidebar, window); + } + finally + { + window.Close(); + } + }, + Token); + private Task OnTheSidebarAsync(Action body) => LayoutHarness.OnTheUiThreadAsync( () => diff --git a/tests/DodoSSH.Client.App.Tests/FakeSsh.cs b/tests/DodoSSH.Client.App.Tests/FakeSsh.cs index fb9c344..6897e4e 100644 --- a/tests/DodoSSH.Client.App.Tests/FakeSsh.cs +++ b/tests/DodoSSH.Client.App.Tests/FakeSsh.cs @@ -29,6 +29,14 @@ internal sealed class FakeSshConnectionFactory : ISshConnectionFactory, ISftpSes /// Requests this factory was asked for, in order. internal List Requests { get; } = []; + /// Every forward started on any connection this factory opened, in order. + internal List Forwards { get; } = []; + + /// + /// Decides whether a forward is refused, and with what sentence, when set. Null from it starts the forward. + /// + internal Func? ForwardFailure { get; set; } + /// /// Runs against each shell request before it succeeds or fails, when set. /// @@ -118,7 +126,7 @@ internal sealed class FakeSshConnectionFactory : ISshConnectionFactory, ISftpSes progress?.Report(SshConnectionPhase.CheckingHostKey); progress?.Report(SshConnectionPhase.Authenticating); - return new FakeSshConnection(request, Shells); + return new FakeSshConnection(request, Shells, this); } /// The file-transfer session this factory last handed out, or null when it never has. @@ -337,7 +345,10 @@ internal sealed class FakeSftpSession(SshConnectionRequest request) : ISftpSessi } } -internal sealed class FakeSshConnection(SshConnectionRequest request, List shells) +internal sealed class FakeSshConnection( + SshConnectionRequest request, + List shells, + FakeSshConnectionFactory factory) : ISshConnection { /// @@ -361,6 +372,20 @@ internal sealed class FakeSshConnection(SshConnectionRequest request, List(shell); } + /// + public Task StartForwardAsync(SshPortForwardSpec spec, CancellationToken cancellationToken) + { + if (factory.ForwardFailure?.Invoke(spec) is { } refusal) + { + return Task.FromException(new SshPortForwardException(refusal)); + } + + var forward = new FakeSshPortForward(spec); + factory.Forwards.Add(forward); + + return Task.FromResult(forward); + } + /// public ValueTask DisposeAsync() { @@ -369,6 +394,43 @@ internal sealed class FakeSshConnection(SshConnectionRequest request, ListA forward that listens on nothing, and can be made to report a failed connection through it. +internal sealed class FakeSshPortForward(SshPortForwardSpec spec) : ISshPortForward +{ + /// + public event EventHandler? ErrorOccurred; + + /// + public SshPortForwardSpec Spec { get; } = spec; + + /// + public int BoundPort => Spec.BindPort; + + /// + public bool IsListening { get; private set; } = true; + + /// + public string? LastError { get; private set; } + + /// Whether whoever started it has stopped it. + internal bool IsDisposed { get; private set; } + + /// Plays SSH.NET reporting a connection through the forward that failed. + internal void Fail(string message) + { + LastError = message; + ErrorOccurred?.Invoke(this, new SshPortForwardErrorEventArgs(message)); + } + + /// + public ValueTask DisposeAsync() + { + IsDisposed = true; + IsListening = false; + return ValueTask.CompletedTask; + } +} + /// A write stream that hands its contents over when it is closed. /// /// A cannot be read after disposal, and disposal is exactly when a write is diff --git a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.Forwards.cs b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.Forwards.cs new file mode 100644 index 0000000..2caa3ef --- /dev/null +++ b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.Forwards.cs @@ -0,0 +1,335 @@ +using DodoSSH.Client.Domain; +using DodoSSH.Client.Shell.ViewModels; +using DodoSSH.Client.Ssh; + +namespace DodoSSH.Client.App.Tests; + +/// +/// Port forwards, from the host editor that saves them to the tab they run on. +/// +/// +/// The listeners here are fakes; PortForwardTests in the SSH suite carries real bytes through all +/// three kinds against a real sshd. What this holds is everything around them: which forwards start, on +/// which session, what a refusal looks like, and that none of it outlives the session. +/// +public sealed partial class ShellFlowTests +{ + [Fact] + public async Task AHostsForwards_StartWithItsTerminal_AndAreCountedInTheStatusBar() + { + var vault = await ConnectedHostWithForwardsAsync(); + + var tab = shell.Tabs.ShouldHaveSingleItem(); + tab.IsLive.ShouldBeTrue(); + + await EventuallyAsync(() => tab.ListeningForwardCount == 2, "both forwards should be listening"); + + tab.Forwards.Select(row => row.Notation).ShouldBe(["-L 5432:db.internal:5432", "-D 1080"]); + tab.ForwardsSummary.ShouldBe("⇄ 2 FORWARDS"); + + ssh.Forwards.Select(forward => forward.Spec).ShouldBe( + [ + new SshPortForwardSpec(SshPortForwardKind.Local, 5432, "db.internal", 5432), + new SshPortForwardSpec(SshPortForwardKind.Dynamic, 1080), + ]); + + vault.SelectedHost.ShouldNotBeNull().HasForwards.ShouldBeTrue(); + } + + [Fact] + public async Task AForwardThatCannotStart_SaysWhy_AndStopsNothingElse() + { + ssh.ForwardFailure = spec => spec.BindPort == 5432 ? "Port 5432 on this machine is already in use." : null; + + await ConnectedHostWithForwardsAsync(); + + var tab = shell.Tabs.ShouldHaveSingleItem(); + + await EventuallyAsync(() => tab.Forwards.All(row => row.State is not PortForwardState.Starting), "both should settle"); + + var refused = tab.Forwards[0]; + refused.State.ShouldBe(PortForwardState.Failed); + refused.StateText.ShouldBe("FAILED"); + refused.Detail.ShouldBe("Port 5432 on this machine is already in use."); + + tab.Forwards[1].IsListening.ShouldBeTrue(); + tab.IsLive.ShouldBeTrue("a forward that cannot start is not a reason to lose the shell"); + tab.ForwardsSummary.ShouldBe("⇄ 1 FORWARD"); + + // The switch on a failed row is a retry, and the port has been freed since. + ssh.ForwardFailure = null; + await refused.ToggleCommand.ExecuteAsync(null); + + refused.IsListening.ShouldBeTrue(); + refused.Detail.ShouldBeNull(); + } + + [Fact] + public async Task AForwardCanBeSwitchedOffAndOnAgain_WithoutTouchingTheSession() + { + await ConnectedHostWithForwardsAsync(); + + var tab = shell.Tabs.ShouldHaveSingleItem(); + await EventuallyAsync(() => tab.ListeningForwardCount == 2, "both forwards should be listening"); + + var row = tab.Forwards[0]; + var first = ssh.Forwards[0]; + + await row.ToggleCommand.ExecuteAsync(null); + + row.State.ShouldBe(PortForwardState.Stopped); + first.IsDisposed.ShouldBeTrue("switching off is what stops the listener"); + tab.ForwardsSummary.ShouldBe("⇄ 1 FORWARD"); + + await row.ToggleCommand.ExecuteAsync(null); + + row.IsListening.ShouldBeTrue(); + ssh.Forwards.Count.ShouldBe(3, "a restart is a new listener on the same session"); + tab.IsLive.ShouldBeTrue(); + } + + [Fact] + public async Task AFailedConnectionThroughAForward_IsShownUnderIt_WhileItKeepsListening() + { + await ConnectedHostWithForwardsAsync(); + + var tab = shell.Tabs.ShouldHaveSingleItem(); + await EventuallyAsync(() => tab.ListeningForwardCount == 2, "both forwards should be listening"); + + ssh.Forwards[0].Fail("Connection refused"); + + tab.Forwards[0].Detail.ShouldBe("Connection refused"); + tab.Forwards[0].IsListening.ShouldBeTrue(); + } + + [Fact] + public async Task WhenTheSessionEnds_ItsForwardsSayTheyWentWithIt() + { + await ConnectedHostWithForwardsAsync(); + + var tab = shell.Tabs.ShouldHaveSingleItem(); + await EventuallyAsync(() => tab.ListeningForwardCount == 2, "both forwards should be listening"); + + ssh.Shells.ShouldHaveSingleItem().Drop("An established connection was aborted by the server."); + + await EventuallyAsync(() => !tab.IsLive, "the dropped shell should have ended the session"); + + tab.Forwards.ShouldAllBe(row => row.State == PortForwardState.Stopped); + tab.Forwards.ShouldAllBe(row => row.Detail == "Stopped with the session."); + tab.Forwards.ShouldAllBe(row => !row.CanToggle); + tab.ForwardsSummary.ShouldBeNull(); + ssh.Forwards.ShouldAllBe(forward => forward.IsDisposed); + } + + [Fact] + public async Task AReconnect_StartsTheForwardsAgainOnTheNewSession() + { + await ConnectedHostWithForwardsAsync(); + + var tab = shell.Tabs.ShouldHaveSingleItem(); + await EventuallyAsync(() => tab.ListeningForwardCount == 2, "both forwards should be listening"); + + var before = tab.Forwards.ToList(); + + ssh.Shells[0].Drop("An established connection was aborted by the server."); + await EventuallyAsync(() => !tab.IsLive, "the dropped shell should have ended the session"); + + await shell.ReconnectTabCommand.ExecuteAsync(tab); + + await EventuallyAsync(() => tab.IsLive && tab.ListeningForwardCount == 2, "the new session's forwards should listen"); + + tab.Forwards.ShouldNotContain(before[0], "a reconnect is a new session, with new rows"); + ssh.Forwards.Count.ShouldBe(4); + } + + [Fact] + public async Task ATypedInAddress_StartsNoForwards() + { + var vault = await ReadyToConnectAsync(); + + await using var renderer = await FakeRenderer.AttachAsync(workspace, Token); + + vault.ManualTarget = "deploy@10.0.0.9"; + vault.ManualPassword = "hunter2"; + await vault.ConnectManuallyCommand.ExecuteAsync(null); + + shell.Tabs.ShouldHaveSingleItem().Forwards.ShouldBeEmpty(); + ssh.Forwards.ShouldBeEmpty(); + } + + // ---- The phone's sheet ---- + + /// + /// The phone lists a tab's forwards in a sheet over the terminal, so it obeys the connect sheet's rules: + /// it collapses the renderer while it is up, and leaving the terminal lowers it however it is left. + /// + [Fact] + public async Task TheForwardsSheet_HidesTheRenderer_AndLeavingTheTerminalLowersIt() + { + await ConnectedHostWithForwardsAsync(); + + shell.SelectedTab.ShouldNotBeNull().HasForwards.ShouldBeTrue(); + + shell.OpenForwardsSheetCommand.Execute(null); + + shell.IsForwardsSheetOpen.ShouldBeTrue(); + shell.IsTerminalShowing.ShouldBeFalse("the sheet draws over the renderer's rectangle"); + shell.IsTerminalSurface.ShouldBeTrue("the bar the sheet was raised from is on that surface"); + + shell.CloseForwardsSheetCommand.Execute(null); + + shell.IsTerminalShowing.ShouldBeTrue(); + + shell.OpenForwardsSheetCommand.Execute(null); + shell.ShowScreenCommand.Execute(ShellScreen.Keychain); + + shell.IsForwardsSheetOpen.ShouldBeFalse("a sheet left up over a page would hold the renderer blank"); + + shell.OpenForwardsSheetCommand.Execute(null); + + shell.IsForwardsSheetOpen.ShouldBeFalse("it must not open over a page either"); + } + + /// + /// What the phone's bar button and its keep-alive notification read: whether a tab has forwards, whether + /// one failed, and how many listen across every tab — with an event each time any of that moves. + /// + [Fact] + public async Task TheCountsAPhoneShows_FollowEveryForwardsState() + { + ssh.ForwardFailure = spec => spec.BindPort == 5432 ? "Port 5432 on this machine is already in use." : null; + + var changes = 0; + shell.ForwardsChanged += (_, _) => changes++; + + await ConnectedHostWithForwardsAsync(); + + var tab = shell.Tabs.ShouldHaveSingleItem(); + await EventuallyAsync(() => tab.Forwards.All(row => row.State is not PortForwardState.Starting), "both should settle"); + + tab.HasForwards.ShouldBeTrue(); + tab.HasFailedForward.ShouldBeTrue(); + tab.HasListeningForward.ShouldBeTrue(); + shell.ListeningForwardCount.ShouldBe(1); + changes.ShouldBeGreaterThan(0); + + var before = changes; + ssh.ForwardFailure = null; + await tab.Forwards[0].ToggleCommand.ExecuteAsync(null); + + tab.HasFailedForward.ShouldBeFalse(); + shell.ListeningForwardCount.ShouldBe(2); + changes.ShouldBeGreaterThan(before, "a retry that listens is a change the notification has to hear"); + + ssh.Shells.ShouldHaveSingleItem().Drop("An established connection was aborted by the server."); + await EventuallyAsync(() => !tab.IsLive, "the dropped shell should have ended the session"); + + shell.ListeningForwardCount.ShouldBe(0); + tab.HasListeningForward.ShouldBeFalse(); + tab.HasForwards.ShouldBeTrue("the rows stay, saying they went with the session"); + } + + // ---- The editor ---- + + [Fact] + public async Task TheEditor_RefusesASecondListenerOnAPortThisHostAlreadyListensOn() + { + var vault = await ReadyToConnectAsync(); + + vault.EditSelectedHostCommand.Execute(null); + + AddForward(vault, PortForwardKind.Local, "8080", "web.internal:80"); + AddForward(vault, PortForwardKind.Dynamic, "8080", string.Empty); + + vault.Status.ShouldBe("This host already has a forward listening on port 8080 here."); + vault.EditorPortForwards.Count.ShouldBe(1); + + // A remote forward listens on the remote, whose ports are a different set. + AddForward(vault, PortForwardKind.Remote, "8080", "localhost:3000"); + + vault.EditorPortForwards.Count.ShouldBe(2); + } + + [Theory] + [InlineData("", "db:5432", "A forward needs a listening port, as a number.")] + [InlineData("-1", "db:5432", "A forward needs a listening port, as a number.")] + [InlineData("70000", "db:5432", "A forward's listening port must be between 1 and 65535, not 70000.")] + [InlineData("5432", "db", "A forward's target is host:port, for example db.internal:5432.")] + [InlineData("5432", "::1:5432", "A forward's target is host:port, for example db.internal:5432.")] + [InlineData("5432", "db:0", "A forward's target port must be between 1 and 65535, not 0.")] + public async Task TheEditor_RefusesAnUnusableForwardAtTheBox(string bindPort, string target, string expected) + { + var vault = await ReadyToConnectAsync(); + + vault.EditSelectedHostCommand.Execute(null); + + AddForward(vault, PortForwardKind.Local, bindPort, target); + + vault.Status.ShouldBe(expected); + vault.EditorPortForwards.ShouldBeEmpty(); + } + + [Theory] + [InlineData("db.internal:5432", "db.internal", 5432)] + [InlineData(" localhost:3000 ", "localhost", 3000)] + [InlineData("[::1]:5432", "::1", 5432)] + [InlineData("10.0.0.9:22", "10.0.0.9", 22)] + public void AForwardTarget_IsReadAsHostColonPort(string text, string host, int port) + { + VaultViewModel.TryParseForwardTarget(text, out var parsedHost, out var parsedPort).ShouldBeTrue(); + + parsedHost.ShouldBe(host); + parsedPort.ShouldBe(port); + } + + [Fact] + public async Task EditingAHost_KeepsItsForwardsIds_SoTheMergeSeesAnEditNotANewForward() + { + var vault = await ReadyToConnectAsync(); + + vault.EditSelectedHostCommand.Execute(null); + AddForward(vault, PortForwardKind.Local, "5432", "db.internal:5432"); + await vault.SaveHostCommand.ExecuteAsync(null); + + var saved = vault.Hosts[0].Host.PortForwards.ShouldHaveSingleItem(); + + vault.SelectedHost = vault.Hosts[0]; + vault.EditSelectedHostCommand.Execute(null); + vault.EditorNotes = "primary"; + await vault.SaveHostCommand.ExecuteAsync(null); + + vault.Hosts[0].Host.PortForwards.ShouldHaveSingleItem().ShouldBe(saved); + } + + /// + /// A host with a bound key and two forwards — one local, one SOCKS — connected, with its tab selected. + /// + private async Task ConnectedHostWithForwardsAsync() + { + var vault = await ReadyToConnectAsync(); + + await AddKeyAsync(vault, "deploy"); + await BindKeyAsync(vault, vault.Hosts[0], vault.Keys[0].EntityId); + + vault.EditSelectedHostCommand.Execute(null); + AddForward(vault, PortForwardKind.Local, "5432", "db.internal:5432"); + AddForward(vault, PortForwardKind.Dynamic, "1080", string.Empty); + vault.EditorPortForwards.Count.ShouldBe(2, vault.Status); + await vault.SaveHostCommand.ExecuteAsync(null); + + await using var renderer = await FakeRenderer.AttachAsync(workspace, Token); + + vault.ChooseHostCommand.Execute(Host(vault, "prod-db")); + await vault.ConnectToChosenHostCommand.ExecuteAsync(null); + + return vault; + } + + private static void AddForward(VaultViewModel vault, PortForwardKind kind, string bindPort, string target) + { + vault.EditorNewForwardKind = (int)kind; + vault.EditorNewForwardBindPort = bindPort; + vault.EditorNewForwardTarget = target; + vault.AddEditorForwardCommand.Execute(null); + } +} diff --git a/tests/DodoSSH.Client.Domain.Tests/HostFactory.cs b/tests/DodoSSH.Client.Domain.Tests/HostFactory.cs index 95a09fb..6c35822 100644 --- a/tests/DodoSSH.Client.Domain.Tests/HostFactory.cs +++ b/tests/DodoSSH.Client.Domain.Tests/HostFactory.cs @@ -21,11 +21,29 @@ internal static class HostFactory /// A second tag id, for the tests about two people tagging one host. internal static Guid EuWest { get; } = Guid.Parse("0192f0c8-9999-7c3d-8e4f-5a6b7c8d9e09"); + /// A forward to the database, as the host would carry it. + internal static PortForwardRule Postgres { get; } = new() + { + Id = Guid.Parse("0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a"), + Kind = PortForwardKind.Local, + BindPort = 5432, + TargetHost = "db.internal", + TargetPort = 5432, + }; + + /// A SOCKS forward, for the tests about two forwards on one host. + internal static PortForwardRule Socks { get; } = new() + { + Id = Guid.Parse("0192f0c8-bbbb-7c3d-8e4f-5a6b7c8d9e0b"), + Kind = PortForwardKind.Dynamic, + BindPort = 1080, + }; + /// A credential holding a Spelunk API key, for the hosts reached through Spelunk. - internal static Guid SpelunkKey { get; } = Guid.Parse("0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a"); + internal static Guid SpelunkKey { get; } = Guid.Parse("0192f0c8-cccc-7c3d-8e4f-5a6b7c8d9e0c"); /// A server's id in Spelunk. - internal static Guid SpelunkServer { get; } = Guid.Parse("0192f0c8-bbbb-7c3d-8e4f-5a6b7c8d9e0b"); + internal static Guid SpelunkServer { get; } = Guid.Parse("0192f0c8-dddd-7c3d-8e4f-5a6b7c8d9e0d"); /// A route to , with . internal static SpelunkRoute ThroughSpelunk(string apiBase = "https://api.spelunk.example.com") => @@ -51,6 +69,7 @@ internal static class HostFactory Guid? groupId = null, Guid[]? tags = null, string[]? pinnedPaths = null, + PortForwardRule[]? portForwards = null, SpelunkRoute? spelunk = null) => new() { @@ -70,6 +89,7 @@ internal static class HostFactory GroupId = groupId, TagIds = tags is null ? TagSet.Empty : TagSet.Create(tags), PinnedPaths = pinnedPaths is null ? PinnedPathList.Empty : PinnedPathList.Create(pinnedPaths), + PortForwards = portForwards is null ? PortForwardList.Empty : PortForwardList.Create(portForwards), Spelunk = spelunk, }; diff --git a/tests/DodoSSH.Client.Domain.Tests/HostSecretCodecTests.cs b/tests/DodoSSH.Client.Domain.Tests/HostSecretCodecTests.cs index 1c18b61..3a94ff0 100644 --- a/tests/DodoSSH.Client.Domain.Tests/HostSecretCodecTests.cs +++ b/tests/DodoSSH.Client.Domain.Tests/HostSecretCodecTests.cs @@ -18,10 +18,10 @@ public sealed class HostSecretCodecTests /// "Full" cannot mean every field: the two authentication bindings are mutually exclusive, so a host may /// carry a key or a credential and never both, and neither may sit beside AsksForPassword. This /// one carries the credential, because that is the newer of the two, plus a group, a pair of tags, a - /// pair of pinned paths and a Spelunk route — which are orthogonal to the binding and are what make this - /// host reach the highest schema version a valid host can. The route and the relay are exclusive in the - /// same way the bindings are, so this carries the newer of those two as well; the relay's flag has its - /// own tests. + /// pair of pinned paths, a pair of forwards and a Spelunk route — which are orthogonal to the binding and + /// are what make this host reach the highest schema version a valid host can. The route and the relay are + /// exclusive in the same way the bindings are, so this carries the newer of those two as well; the relay's + /// flag has its own tests. /// [Fact] public void AFullHost_RoundTrips() @@ -40,6 +40,7 @@ public sealed class HostSecretCodecTests groupId: Production, tags: [Pci, EuWest], pinnedPaths: ["/var/www/app", "/var/log"], + portForwards: [Postgres, Socks], spelunk: ThroughSpelunk()); HostSecretCodec.TryDecode(HostSecretCodec.Encode(host), out var document).ShouldBeTrue(); diff --git a/tests/DodoSSH.Client.Domain.Tests/HostSecretMergeTests.cs b/tests/DodoSSH.Client.Domain.Tests/HostSecretMergeTests.cs index d581df8..841e8ae 100644 --- a/tests/DodoSSH.Client.Domain.Tests/HostSecretMergeTests.cs +++ b/tests/DodoSSH.Client.Domain.Tests/HostSecretMergeTests.cs @@ -59,6 +59,7 @@ public sealed class HostSecretMergeTests GroupId = Production, TagIds = TagSet.Create([Pci]), PinnedPaths = PinnedPathList.Create(["/var/www/app"]), + PortForwards = PortForwardList.Create([Postgres]), }; var result = HostSecretMerge.Merge(ancestor, local, ancestor); diff --git a/tests/DodoSSH.Client.Domain.Tests/PortForwardTests.cs b/tests/DodoSSH.Client.Domain.Tests/PortForwardTests.cs new file mode 100644 index 0000000..a6b8ef2 --- /dev/null +++ b/tests/DodoSSH.Client.Domain.Tests/PortForwardTests.cs @@ -0,0 +1,225 @@ +using System.Text; +using static DodoSSH.Client.Domain.Tests.HostFactory; + +namespace DodoSSH.Client.Domain.Tests; + +/// Port forwards as a host carries them: stored, validated, compared and merged. +public sealed class PortForwardTests +{ + // ---- Encoding ---- + + [Fact] + public void AHostWithForwards_IsWrittenAtTheirVersion_AndOnlyThen() + { + HostSecretCodec.TryDecode(HostSecretCodec.Encode(Host(portForwards: [Postgres])), out var with) + .ShouldBeTrue(); + HostSecretCodec.TryDecode(HostSecretCodec.Encode(Host(portForwards: [])), out var without) + .ShouldBeTrue(); + + with.ShouldNotBeNull().SchemaVersion.ShouldBe(HostSecretCodec.PortForwardsSchemaVersion); + without.ShouldNotBeNull().SchemaVersion.ShouldBe(HostSecretCodec.BaseSchemaVersion); + } + + [Fact] + public void Forwards_AreWrittenLast_InListOrder_WithTheKindAsItsNumber() + { + // Last, so every older field keeps its bytes. The kind is a number so that renaming an enum member + // can never change what another machine reads. + var bytes = HostSecretCodec.Encode(Host(username: null, portForwards: [Socks, Postgres])); + + Encoding.UTF8.GetString(bytes).ShouldBe( + """ + {"schemaVersion":8,"label":"prod-db","hostname":"db.internal","port":22,"jumpHostIds":[],"options":{},"relayEnabled":false,"portForwards":[{"id":"0192f0c8-bbbb-7c3d-8e4f-5a6b7c8d9e0b","kind":2,"bindPort":1080},{"id":"0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a","kind":0,"bindPort":5432,"targetHost":"db.internal","targetPort":5432}]} + """); + } + + [Fact] + public void APayloadFromBeforeForwards_DecodesAsCarryingNone() + { + var payload = Encoding.UTF8.GetBytes( + """ + {"schemaVersion":7,"label":"prod-db","hostname":"db.internal","port":22,"pinnedPaths":["/var/log"]} + """); + + HostSecretCodec.TryDecode(payload, out var document).ShouldBeTrue(); + + document.ShouldNotBeNull().Host.PortForwards.ShouldBe(PortForwardList.Empty); + document.IsReadOnly.ShouldBeFalse(); + } + + [Theory] + [InlineData("""{"kind":0,"bindPort":5432,"targetHost":"db","targetPort":5432}""")] + [InlineData("""{"id":"0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a","bindPort":5432,"targetHost":"db","targetPort":5432}""")] + [InlineData("""{"id":"0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a","kind":0,"targetHost":"db","targetPort":5432}""")] + [InlineData("""{"id":"0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a","kind":7,"bindPort":5432,"targetHost":"db","targetPort":5432}""")] + [InlineData("""{"id":"0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a","kind":0,"bindPort":5432}""")] + [InlineData("""{"id":"0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a","kind":2,"bindPort":1080,"targetHost":"db","targetPort":5432}""")] + [InlineData("null")] + public void AForwardMissingOrContradictingWhatItNeeds_FailsTheDecode(string forward) + { + // Nothing is filled in with a default: an invented id or kind is a forward the next save writes back + // as though somebody had asked for it. + var payload = Encoding.UTF8.GetBytes( + $$"""{"schemaVersion":8,"label":"prod-db","hostname":"db.internal","port":22,"portForwards":[{{forward}}]}"""); + + HostSecretCodec.TryDecode(payload, out _).ShouldBeFalse(); + } + + [Fact] + public void APayloadRepeatingAForwardId_DecodesKeepingTheFirst() + { + var payload = Encoding.UTF8.GetBytes( + """ + {"schemaVersion":8,"label":"prod-db","hostname":"db.internal","port":22,"portForwards":[{"id":"0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a","kind":0,"bindPort":5432,"targetHost":"db.internal","targetPort":5432},{"id":"0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a","kind":2,"bindPort":1080}]} + """); + + HostSecretCodec.TryDecode(payload, out var document).ShouldBeTrue(); + + document.ShouldNotBeNull().Host.PortForwards.ShouldBe(PortForwardList.Create([Postgres])); + } + + // ---- Validation ---- + + [Fact] + public void AHostCarryingTooManyForwards_IsRefused() + { + var forwards = Enumerable.Range(0, HostSecret.MaxPortForwards + 1) + .Select(i => Postgres with { Id = Guid.CreateVersion7(), BindPort = 10000 + i }) + .ToArray(); + + Host(portForwards: forwards).TryValidate(out var reason).ShouldBeFalse(); + reason.ShouldBe($"A host cannot carry more than {HostSecret.MaxPortForwards} port forwards."); + } + + [Fact] + public void TwoForwardsOnOnePort_AreStorable() + { + // What two machines each adding one produces, and a merge whose result could not be written back + // would strand the host. The second listener is what fails, when it starts. + var clash = Postgres with { Id = Guid.CreateVersion7(), TargetHost = "replica.internal" }; + + Host(portForwards: [Postgres, clash]).TryValidate(out _).ShouldBeTrue(); + } + + [Theory] + [InlineData(PortForwardKind.Local, 0, "db", 5432, "A forward's listening port must be between 1 and 65535, not 0.")] + [InlineData(PortForwardKind.Local, 5432, " ", 5432, "A forward needs a host to forward to.")] + [InlineData(PortForwardKind.Local, 5432, "db internal", 5432, "A forward's target cannot contain spaces or control characters.")] + [InlineData(PortForwardKind.Remote, 8080, "localhost", null, "A forward needs a port to forward to.")] + [InlineData(PortForwardKind.Remote, 8080, "localhost", 70000, "A forward's target port must be between 1 and 65535, not 70000.")] + [InlineData(PortForwardKind.Dynamic, 1080, "db", null, "A dynamic forward has no target: each connection through it names its own.")] + public void AnInvalidForward_IsRefusedWithItsReason( + PortForwardKind kind, + int bindPort, + string? targetHost, + int? targetPort, + string expected) + { + var forward = new PortForwardRule + { + Id = Guid.CreateVersion7(), + Kind = kind, + BindPort = bindPort, + TargetHost = targetHost, + TargetPort = targetPort, + }; + + forward.TryValidate(out var reason).ShouldBeFalse(); + reason.ShouldBe(expected); + + Host(portForwards: [forward]).TryValidate(out _).ShouldBeFalse(); + } + + [Fact] + public void AnEmptyId_IsRefused() + { + (Postgres with { Id = Guid.Empty }).TryValidate(out var reason).ShouldBeFalse(); + reason.ShouldBe("A port forward cannot have an empty id."); + } + + [Theory] + [InlineData(PortForwardKind.Local, "-L 5432:db.internal:5432")] + [InlineData(PortForwardKind.Remote, "-R 5432:db.internal:5432")] + public void Notation_IsOpenSshs(PortForwardKind kind, string expected) + { + (Postgres with { Kind = kind }).Notation.ShouldBe(expected); + Socks.Notation.ShouldBe("-D 1080"); + } + + // ---- Equality ---- + + [Fact] + public void AForwardList_ComparesByContentsAndOrder() + { + PortForwardList.Create([Postgres, Socks]).ShouldBe(PortForwardList.Create([Postgres, Socks])); + (PortForwardList.Create([Postgres, Socks]) == PortForwardList.Create([Postgres, Socks])).ShouldBeTrue(); + PortForwardList.Create([Postgres, Socks]).ShouldNotBe(PortForwardList.Create([Socks, Postgres])); + Host(portForwards: [Postgres]).ShouldBe(Host(portForwards: [Postgres])); + } + + // ---- Merge ---- + + [Fact] + public void TwoMachinesEachAddingAForward_BothKeepTheirs() + { + var ancestor = Host(); + + var result = HostSecretMerge.Merge( + ancestor, + ancestor with { PortForwards = PortForwardList.Create([Postgres]) }, + ancestor with { PortForwards = PortForwardList.Create([Socks]) }); + + result.Merged.PortForwards.ShouldBe(PortForwardList.Create([Postgres, Socks])); + result.HasConflicts.ShouldBeFalse(); + } + + [Fact] + public void OneForwardEditedDifferentlyOnBothSides_TakesTheServersAndRecordsTheOther() + { + var ancestor = Host(portForwards: [Postgres]); + var local = Postgres with { BindPort = 15432 }; + var remote = Postgres with { TargetHost = "replica.internal" }; + + var result = HostSecretMerge.Merge( + ancestor, + ancestor with { PortForwards = PortForwardList.Create([local]) }, + ancestor with { PortForwards = PortForwardList.Create([remote]) }); + + result.Merged.PortForwards.ShouldBe(PortForwardList.Create([remote])); + + var conflict = result.Conflicts.ShouldHaveSingleItem(); + conflict.Field.ShouldBe($"PortForwards[{Postgres.Id}]"); + conflict.Kept.ShouldBe("-L 5432:replica.internal:5432"); + conflict.Discarded.ShouldBe("-L 15432:db.internal:5432"); + conflict.DiscardedWasRemoval.ShouldBeFalse(); + } + + [Fact] + public void AnEditBeatsARemoval_AndTheRemovalIsRecorded() + { + var ancestor = Host(portForwards: [Postgres]); + var edited = Postgres with { BindPort = 15432 }; + + var result = HostSecretMerge.Merge( + ancestor, + ancestor with { PortForwards = PortForwardList.Empty }, + ancestor with { PortForwards = PortForwardList.Create([edited]) }); + + result.Merged.PortForwards.ShouldBe(PortForwardList.Create([edited])); + result.Conflicts.ShouldHaveSingleItem().DiscardedWasRemoval.ShouldBeTrue(); + } + + [Fact] + public void ARemovalOnOneSideOnly_IsTaken() + { + var ancestor = Host(portForwards: [Postgres, Socks]); + + var result = HostSecretMerge.Merge( + ancestor, + ancestor with { PortForwards = PortForwardList.Create([Socks]) }, + ancestor); + + result.Merged.PortForwards.ShouldBe(PortForwardList.Create([Socks])); + result.HasConflicts.ShouldBeFalse(); + } +} diff --git a/tests/DodoSSH.Client.Domain.Tests/SpelunkRouteTests.cs b/tests/DodoSSH.Client.Domain.Tests/SpelunkRouteTests.cs index f10461f..42b6199 100644 --- a/tests/DodoSSH.Client.Domain.Tests/SpelunkRouteTests.cs +++ b/tests/DodoSSH.Client.Domain.Tests/SpelunkRouteTests.cs @@ -122,7 +122,7 @@ public sealed class SpelunkRouteTests Encoding.UTF8.GetString(bytes).ShouldBe( """ - {"schemaVersion":8,"label":"prod-db","hostname":"db.internal","port":22,"jumpHostIds":[],"options":{},"relayEnabled":false,"spelunk":{"api":"https://api.spelunk.example.com","serverId":"0192f0c8-bbbb-7c3d-8e4f-5a6b7c8d9e0b","apiKeyCredentialId":"0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a"}} + {"schemaVersion":9,"label":"prod-db","hostname":"db.internal","port":22,"jumpHostIds":[],"options":{},"relayEnabled":false,"spelunk":{"api":"https://api.spelunk.example.com","serverId":"0192f0c8-dddd-7c3d-8e4f-5a6b7c8d9e0d","apiKeyCredentialId":"0192f0c8-cccc-7c3d-8e4f-5a6b7c8d9e0c"}} """); } @@ -146,7 +146,7 @@ public sealed class SpelunkRouteTests // The other reading — no route — would turn a damaged Spelunk host into one dialled directly. var payload = Encoding.UTF8.GetBytes( """ - {"schemaVersion":8,"label":"prod-db","hostname":"db.internal","port":22,"spelunk":{"api":"https://api.spelunk.example.com"}} + {"schemaVersion":9,"label":"prod-db","hostname":"db.internal","port":22,"spelunk":{"api":"https://api.spelunk.example.com"}} """); HostSecretCodec.TryDecode(payload, out _).ShouldBeFalse(); @@ -156,10 +156,10 @@ public sealed class SpelunkRouteTests public void ARouteWrittenByANewerClient_IsReadOnlyHere() { // The rule every version before this one relies on, checked from the far side: a build that knows - // version 8 reads a version 9 host, shows it, and refuses to re-encode what it cannot represent. + // version 9 reads a version 10 host, shows it, and refuses to re-encode what it cannot represent. var payload = Encoding.UTF8.GetBytes( """ - {"schemaVersion":9,"label":"prod-db","hostname":"db.internal","port":22,"spelunk":{"api":"https://api.spelunk.example.com","serverId":"0192f0c8-bbbb-7c3d-8e4f-5a6b7c8d9e0b","apiKeyCredentialId":"0192f0c8-aaaa-7c3d-8e4f-5a6b7c8d9e0a"},"somethingNewer":true} + {"schemaVersion":10,"label":"prod-db","hostname":"db.internal","port":22,"spelunk":{"api":"https://api.spelunk.example.com","serverId":"0192f0c8-dddd-7c3d-8e4f-5a6b7c8d9e0d","apiKeyCredentialId":"0192f0c8-cccc-7c3d-8e4f-5a6b7c8d9e0c"},"somethingNewer":true} """); HostSecretCodec.TryDecode(payload, out var document).ShouldBeTrue(); diff --git a/tests/DodoSSH.Client.Ssh.Tests/PortForwardTests.cs b/tests/DodoSSH.Client.Ssh.Tests/PortForwardTests.cs new file mode 100644 index 0000000..259306d --- /dev/null +++ b/tests/DodoSSH.Client.Ssh.Tests/PortForwardTests.cs @@ -0,0 +1,252 @@ +using System.Net; +using System.Net.Sockets; +using System.Security.Cryptography; +using System.Text; + +namespace DodoSSH.Client.Ssh.Tests; + +/// +/// The three kinds of forward, each carrying real bytes through the container's sshd. +/// +/// +/// +/// ◆ The target of a local forward is named as the container sees it, for the reason +/// LoopbackProxyTests gives: the remote resolves it, so localhost:2222 is the container's own +/// sshd, and the published port this test host uses means nothing in there. What comes back is that sshd's +/// banner, which is bytes nothing but a real forward could have fetched. +/// +/// +/// A remote forward is proved the other way round. A listener on this test host answers with a known line, +/// the remote is asked to listen for it, and a local forward on the same connection reaches that remote +/// port from the inside — so the line has crossed the connection twice, once in each direction. +/// +/// +[Collection(SshCollection.Name)] +public sealed class PortForwardTests(SshServerFixture fixture) +{ + /// What the container calls itself. See the class remarks. + private const string InternalHost = "localhost"; + + private static CancellationToken Token => TestContext.Current.CancellationToken; + + [Fact] + public async Task ALocalForward_CarriesAConnectionToATargetOnlyTheRemoteCanSee() + { + await using var connection = await ConnectAsync(); + + await using var forward = await connection.StartForwardAsync( + new SshPortForwardSpec(SshPortForwardKind.Local, 0, InternalHost, SshServerFixture.InternalPort), + Token); + + forward.IsListening.ShouldBeTrue(); + forward.BoundPort.ShouldBeGreaterThan(0, "an ephemeral port is reported back once bound"); + + using var client = new TcpClient(); + await client.ConnectAsync(IPAddress.Loopback, forward.BoundPort, Token); + + (await ReadLineAsync(client.GetStream())).ShouldStartWith("SSH-2.0-"); + } + + [Fact] + public async Task ADynamicForward_IsASocksProxyWhoseConnectionsLeaveFromTheRemote() + { + await using var connection = await ConnectAsync(); + + await using var forward = await connection.StartForwardAsync( + new SshPortForwardSpec(SshPortForwardKind.Dynamic, 0), Token); + + using var client = new TcpClient(); + await client.ConnectAsync(IPAddress.Loopback, forward.BoundPort, Token); + + var stream = client.GetStream(); + await Socks5ConnectAsync(stream, InternalHost, SshServerFixture.InternalPort); + + (await ReadLineAsync(stream)).ShouldStartWith("SSH-2.0-"); + } + + [Fact] + public async Task ARemoteForward_CarriesAConnectionOnTheRemoteBackToThisMachine() + { + const string Greeting = "hello from the client side"; + + using var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + + var served = ServeOneLineAsync(listener, Greeting); + + await using var connection = await ConnectAsync(); + + var remotePort = RandomNumberGenerator.GetInt32(20000, 60000); + + await using var remote = await connection.StartForwardAsync( + new SshPortForwardSpec( + SshPortForwardKind.Remote, + remotePort, + "127.0.0.1", + ((IPEndPoint)listener.LocalEndpoint).Port), + Token); + + remote.BoundPort.ShouldBe(remotePort); + + // Reaches the remote's own loopback, which is where the remote forward is listening. + await using var inward = await connection.StartForwardAsync( + new SshPortForwardSpec(SshPortForwardKind.Local, 0, "127.0.0.1", remotePort), + Token); + + using var client = new TcpClient(); + await client.ConnectAsync(IPAddress.Loopback, inward.BoundPort, Token); + + (await ReadLineAsync(client.GetStream())).ShouldBe(Greeting); + + await served; + } + + [Fact] + public async Task APortAlreadyInUse_IsRefusedWithASentenceNamingIt() + { + using var squatter = new TcpListener(IPAddress.Loopback, 0); + squatter.Start(); + + var taken = ((IPEndPoint)squatter.LocalEndpoint).Port; + + await using var connection = await ConnectAsync(); + + var refused = await Should.ThrowAsync(async () => + await connection.StartForwardAsync( + new SshPortForwardSpec(SshPortForwardKind.Local, taken, InternalHost, SshServerFixture.InternalPort), + Token)); + + refused.Message.ShouldBe($"Port {taken} on this machine is already in use."); + + // And the refusal left nothing half-registered on the client: the next forward still starts. + await using var next = await connection.StartForwardAsync( + new SshPortForwardSpec(SshPortForwardKind.Local, 0, InternalHost, SshServerFixture.InternalPort), + Token); + + next.IsListening.ShouldBeTrue(); + } + + [Fact] + public async Task DisposingAForward_StopsItsListener() + { + await using var connection = await ConnectAsync(); + + var forward = await connection.StartForwardAsync( + new SshPortForwardSpec(SshPortForwardKind.Local, 0, InternalHost, SshServerFixture.InternalPort), + Token); + + var port = forward.BoundPort; + + await forward.DisposeAsync(); + + forward.IsListening.ShouldBeFalse(); + await ShouldRefuseAsync(port); + + // Idempotent, because the connection closing will try again. + await forward.DisposeAsync(); + } + + [Fact] + public async Task ClosingTheConnection_StopsEveryForwardOnIt() + { + var connection = await ConnectAsync(); + + var forward = await connection.StartForwardAsync( + new SshPortForwardSpec(SshPortForwardKind.Dynamic, 0), Token); + + var port = forward.BoundPort; + + await connection.DisposeAsync(); + + forward.IsListening.ShouldBeFalse(); + await ShouldRefuseAsync(port); + + // What the workspace does after closing a session, and it must be harmless. + await forward.DisposeAsync(); + } + + [Fact] + public async Task AnInvalidSpec_IsRefusedBeforeAnythingListens() + { + await using var connection = await ConnectAsync(); + + await Should.ThrowAsync(async () => + await connection.StartForwardAsync( + new SshPortForwardSpec(SshPortForwardKind.Local, 0, TargetHost: null, TargetPort: 0), + Token)); + + await Should.ThrowAsync(async () => + await connection.StartForwardAsync( + new SshPortForwardSpec(SshPortForwardKind.Remote, 0, "127.0.0.1", 80), + Token)); + } + + private static async Task ShouldRefuseAsync(int port) + { + using var client = new TcpClient(); + + await Should.ThrowAsync(async () => + await client.ConnectAsync(IPAddress.Loopback, port, Token)); + } + + private static async Task ServeOneLineAsync(TcpListener listener, string line) + { + using var accepted = await listener.AcceptTcpClientAsync(Token); + + await accepted.GetStream().WriteAsync(Encoding.ASCII.GetBytes(line + "\n"), Token); + } + + /// + /// The smallest SOCKS5 exchange there is: no authentication, then CONNECT by name. + /// + private static async Task Socks5ConnectAsync(NetworkStream stream, string host, int port) + { + await stream.WriteAsync(new byte[] { 5, 1, 0 }, Token); + + var method = new byte[2]; + await stream.ReadExactlyAsync(method, Token); + method.ShouldBe(new byte[] { 5, 0 }); + + var name = Encoding.ASCII.GetBytes(host); + byte[] request = [5, 1, 0, 3, (byte)name.Length, .. name, (byte)(port >> 8), (byte)port]; + await stream.WriteAsync(request, Token); + + // An IPv4 bound address, which is what SSH.NET always answers with: ten bytes. + var reply = new byte[10]; + await stream.ReadExactlyAsync(reply, Token); + reply[1].ShouldBe((byte)0, "the proxy should report the CONNECT as granted"); + } + + private static async Task ReadLineAsync(NetworkStream stream) + { + var line = new List(); + var one = new byte[1]; + + using var timeout = CancellationTokenSource.CreateLinkedTokenSource(Token); + timeout.CancelAfter(TimeSpan.FromSeconds(15)); + + while (await stream.ReadAsync(one, timeout.Token) == 1 && one[0] != (byte)'\n') + { + line.Add(one[0]); + } + + return Encoding.ASCII.GetString([.. line]).TrimEnd('\r'); + } + + private SshConnectionRequest Request() => + new(fixture.Host, fixture.Port, SshServerFixture.Username, new SshPasswordCredential(SshServerFixture.Password)); + + /// Connects with the container's host key already trusted, since first contact is not the subject. + private async Task ConnectAsync() + { + var knownHosts = new InMemoryKnownHostStore(); + var factory = new SshNetConnectionFactory(knownHosts); + + var unknown = await Should.ThrowAsync(async () => + await factory.ConnectAsync(Request(), progress: null, Token)); + + await knownHosts.TrustAsync(unknown.Presentation, Token); + + return await factory.ConnectAsync(Request(), progress: null, Token); + } +} diff --git a/tests/DodoSSH.Client.Sync.Tests/ItemKindsTests.cs b/tests/DodoSSH.Client.Sync.Tests/ItemKindsTests.cs index 6f853e0..79acabd 100644 --- a/tests/DodoSSH.Client.Sync.Tests/ItemKindsTests.cs +++ b/tests/DodoSSH.Client.Sync.Tests/ItemKindsTests.cs @@ -1,3 +1,4 @@ +using DodoSSH.Client.Domain; using DodoSSH.Contracts; namespace DodoSSH.Client.Sync.Tests; @@ -53,4 +54,23 @@ public sealed class ItemKindsTests reconcilers.Keys.Order().ShouldBe(ItemKinds.SyncedTypes.Order()); } + + [Fact] + public void AHostEditThatOnlyChangedItsPins_IsReportedAsChangingPinnedPaths() + { + // Pinned paths were the one host field Changes did not look at, so an edit that only pinned or + // unpinned a directory reached the keychain log as an update that changed nothing. + var before = new HostSecret + { + Label = "db-01", + Hostname = "db-01.internal", + PinnedPaths = PinnedPathList.Create(["/var/log"]), + }; + var after = before with { PinnedPaths = PinnedPathList.Create(["/var/log", "/etc/nginx"]) }; + + HostKind.Instance.Changes(before, after).ShouldBe(["Pinned paths"]); + HostKind.Instance.Changes(before, before with { PinnedPaths = PinnedPathList.Empty }) + .ShouldBe(["Pinned paths"]); + HostKind.Instance.Changes(before, before with { }).ShouldBeEmpty(); + } } diff --git a/tests/DodoSSH.Client.Terminal.Tests/FakeShellSession.cs b/tests/DodoSSH.Client.Terminal.Tests/FakeShellSession.cs index 430ca84..a9df86a 100644 --- a/tests/DodoSSH.Client.Terminal.Tests/FakeShellSession.cs +++ b/tests/DodoSSH.Client.Terminal.Tests/FakeShellSession.cs @@ -224,6 +224,17 @@ internal sealed class FakeConnection(SshConnectionRequest request, long bytesPer return Task.FromResult(Shell); } + /// The forwards started on this connection, in order. + internal List Forwards { get; } = []; + + /// + public Task StartForwardAsync(SshPortForwardSpec spec, CancellationToken cancellationToken) + { + Forwards.Add(spec); + + return Task.FromResult(new FakePortForward(spec)); + } + /// public ValueTask DisposeAsync() { @@ -361,3 +372,33 @@ internal sealed class DelegateProgress(Action report) : IProgress { public void Report(T value) => report(value); } + +/// A forward that listens on nothing and says it is listening until disposed. +internal sealed class FakePortForward(SshPortForwardSpec spec) : ISshPortForward +{ + /// + public event EventHandler? ErrorOccurred + { + add { } + remove { } + } + + /// + public SshPortForwardSpec Spec { get; } = spec; + + /// + public int BoundPort => Spec.BindPort; + + /// + public bool IsListening { get; private set; } = true; + + /// + public string? LastError => null; + + /// + public ValueTask DisposeAsync() + { + IsListening = false; + return ValueTask.CompletedTask; + } +} diff --git a/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs b/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs index 5f2eaa4..a4f11db 100644 --- a/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs +++ b/tests/DodoSSH.Client.Terminal.Tests/TerminalWorkspaceTests.cs @@ -599,6 +599,53 @@ public sealed class TerminalWorkspaceTests await WaitUntilAsync(() => Volatile.Read(ref reattachedCount) > 0); } + // ---- Port forwards ---- + + [Fact] + public async Task AForward_RunsOverTheConnectionOfTheSessionItNames() + { + var connections = new FakeConnectionFactory(blockShellReads: true); + + await using var workspace = CreateWorkspace(connections); + + await workspace.OpenSessionAsync( + Request(), TerminalSize.Default, progress: null, TestContext.Current.CancellationToken); + var second = await workspace.OpenSessionAsync( + Request(), TerminalSize.Default, progress: null, TestContext.Current.CancellationToken); + + var spec = new SshPortForwardSpec(SshPortForwardKind.Local, 5432, "db.internal", 5432); + + await using var forward = await workspace.StartForwardAsync( + second, spec, TestContext.Current.CancellationToken); + + forward.Spec.ShouldBe(spec); + connections.Connections[0].Forwards.ShouldBeEmpty(); + connections.Connections[1].Forwards.ShouldBe([spec]); + } + + [Fact] + public async Task AForwardOnASessionThatIsNotRunning_IsRefused() + { + var connections = new FakeConnectionFactory(bytesPerShell: 0); + + await using var workspace = CreateWorkspace(connections); + + var ended = await workspace.OpenSessionAsync( + Request(), TerminalSize.Default, progress: null, TestContext.Current.CancellationToken); + + await WaitUntilAsync(() => !workspace.IsSessionLive(ended)); + + var spec = new SshPortForwardSpec(SshPortForwardKind.Dynamic, 1080); + + await Should.ThrowAsync(async () => + await workspace.StartForwardAsync(ended, spec, TestContext.Current.CancellationToken)); + + await Should.ThrowAsync(async () => + await workspace.StartForwardAsync(999, spec, TestContext.Current.CancellationToken)); + + connections.Connections[0].Forwards.ShouldBeEmpty(); + } + // ---- Helpers ---- ///