diff --git a/CHANGELOG.md b/CHANGELOG.md index f080b349..ee9fe420 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Versions follow [Semantic Versioning](https://semver.org/). - **Drag files and folders between the SFTP panes, or onto the remote pane from your file manager.** In the desktop app, dragging a row to the other pane uploads or downloads it into the folder row it is dropped on, or else into the folder on show, and dragging a ticked row takes every ticked row along. Files and folders dropped from Explorer, Finder or a Linux file manager onto the remote pane upload to it. Escape cancels a drag. - **Both apps transfer whole folders.** Upload and Download in the desktop app, and copy and paste in the terminal app, take a folder with everything in it, merge it into a folder of the same name and replace files that clash, and the desktop app asks first when the folder on show already has those names. Files and folders keep their permission bits, so scripts stay executable, and a copy stays writable for you so the next transfer can replace it; symbolic links and special files are skipped, and the transfer ends by saying how many items were skipped or failed and why the first one was. Cancel in the desktop app's progress strip, closing its tab, or Esc in the terminal app stops a transfer and removes the file it was part way through. - **The file panes show when each file was last changed.** The desktop app's panes gain a Modified column, with the full date and time on hover, once the window is wide enough. The terminal app's file manager shows the time at the end of each row when its panel is wide enough. +- **Streamer mode shows when it is on and switches from anywhere.** In the desktop app it could only be switched in Settings, and nothing on screen said it was on. While it is on, an eye in the status bar says so. `⌘⇧S` on macOS or `Ctrl+Shift+S` elsewhere and "Toggle streamer mode" in the command palette switch it from any screen, a terminal included. ### Bug Fixes - **Security: a server that offers a host key of another type than the one saved for it is refused.** OmnySSH compared the offered key only with saved keys of the same type. A host saved with its Ed25519 key that suddenly offered only an RSA or ECDSA key was therefore taken for a new host: the key was saved and the login went ahead, password included, and the dashboard did this in the background without anyone connecting. Someone intercepting the connection could get past the saved key this way. Like `ssh`, OmnySSH now refuses any key that matches none of those saved for the host, whatever its type, and says which type is saved. A server that really dropped the type of key it was saved with needs its old key removed once, as `ssh` also asks. @@ -33,6 +34,11 @@ Versions follow [Semantic Versioning](https://semver.org/). - **Pasting a folder in the terminal app copies it instead of leaving an empty file.** The file manager handed a folder to the transfer as if it were a file, so the paste failed after creating an empty file of that name on the other side. - **A paste or delete of more than 64 items in the terminal app handles them all.** They went to the SFTP queue at once, and whatever did not fit in its 64 places was dropped without a word while the app kept waiting for it. They now go one at a time. - **The terminal app no longer crashes when a file grows while it is copied.** A log still being written to could end up larger than when its transfer started, which took the progress bar past 100% and brought the app down. +- **Key setup no longer reports success while the server still takes passwords.** Turning password login off needs sudo without a password prompt. Without it, as for an account whose sudo asks for a password, key setup stopped once the new key worked, yet the desktop app said "Key authentication configured", the card showed a key badge and the key button was gone, so there was no way to try again. The terminal app went further: it marked the host key-only and deleted the saved password. Both apps now say that key login works and password login is still on, and keep the saved password. The desktop app's card shows a "Password on" badge that runs the setup again, for when sudo allows it, and a shield for a host that takes keys only. After a full setup it also says that the server's SSH settings were changed, where their backup is, and that the saved password was removed. +- **The desktop app asks before setting up a key.** One click on the key icon generated a key, added it to the server and could turn password login off for every account on it. A dialog now says what will happen and names the key file, and nothing starts until you confirm. Cancel is focused, and Esc or a click outside cancels. +- **Key setup that fails after turning password login off says so.** When the last check failed, the desktop app said "Password authentication was not changed", although password login had been turned off, and even when turning it back on failed and left the server reachable only through the hosting provider's console. It now says when password login was turned back on, and when it may still be off, now or after the SSH service restarts: then it keeps the new key for the host, says how to check the server's setting and restore its backup, and offers a terminal to do it. +- **The desktop app asks before quitting while anything is still open.** Closing the window, `⌘Q`, the tray's Quit, and `⌘W` on macOS ended every terminal, transfer, tunnel and snippet run without a word. Closing or quitting now asks first and names what will be closed, with a "Don't ask again" box; Settings → Window turns the question back on. On macOS `⌘W` closes the active tab and `⌘⇧W` the window. Quitting from the Dock or by logging out still does not ask. +- **Streamer mode hides addresses in error messages and the host form too.** The cards showed made-up addresses, but the status bar printed real ones, for example for a changed host key or a tunnel that could not open, and so did the file panes, snippet results and key setup. Errors now show the same made-up addresses as the cards, and the host form hides its addresses until you click Reveal. --- diff --git a/crates/omnyssh-core/src/event.rs b/crates/omnyssh-core/src/event.rs index b193d472..db0923cc 100644 --- a/crates/omnyssh-core/src/event.rs +++ b/crates/omnyssh-core/src/event.rs @@ -149,6 +149,12 @@ pub enum CoreEvent { KeySetupFailed(HostId, String), /// Emergency rollback was triggered (host_id, rollback_result). KeySetupRollback(HostId, String), + /// Key login works, but password login stayed on: changing the server's + /// settings needs sudo without a password (host_id, private_key_path). + KeySetupPartial(HostId, std::path::PathBuf), + /// Key setup failed and the server's password login may be off: the + /// rollback failed or the disable step never answered (host_id, error_message). + KeySetupFailedUnsafe(HostId, String), /// A private key is encrypted and no passphrase is cached for it yet. /// Frontends prompt once per key path and call [`crate::ssh::identity::unlock`]. diff --git a/crates/omnyssh-core/src/ssh/key_setup.rs b/crates/omnyssh-core/src/ssh/key_setup.rs index 9a24ed33..6069d592 100644 --- a/crates/omnyssh-core/src/ssh/key_setup.rs +++ b/crates/omnyssh-core/src/ssh/key_setup.rs @@ -125,6 +125,9 @@ pub struct KeySetupMachine { current_step: Option, has_sudo: bool, password_disabled: bool, + /// The step-4 command was sent and has not answered. Kept on the machine + /// so it survives the total timeout dropping the running future. + disable_unanswered: bool, } impl KeySetupMachine { @@ -135,6 +138,7 @@ impl KeySetupMachine { current_step: None, has_sudo: true, password_disabled: false, + disable_unanswered: false, } } @@ -208,6 +212,35 @@ impl KeySetupMachine { self.state = KeySetupState::RolledBack; self.password_disabled = false; } + + /// Records that the step-4 command is about to run. + fn disable_sent(&mut self) { + self.disable_unanswered = true; + } + + /// Records that the step-4 command answered, so its effect is known. + fn disable_answered(&mut self) { + self.disable_unanswered = false; + } + + /// Fails the run on the total timeout. + fn timed_out(&mut self) { + // Running out of time is only safe before the point of no return. + // Past it the server has password auth disabled, so the run needs + // the same rollback a failed final check would get. + let step = if self.password_disabled { + KeySetupStep::FinalCheck + } else { + KeySetupStep::VerifyKeyAuth + }; + self.step_result(step, Err(anyhow!("timed out"))); + } + + /// Whether a failed run, once any rollback was tried, may have left + /// password login off: the rollback failed, or step 4 never answered. + fn password_may_be_off(&self) -> bool { + self.disable_unanswered || self.state == KeySetupState::NeedsRollback + } } impl Default for KeySetupMachine { @@ -488,13 +521,15 @@ pub fn build_reload_sshd_command() -> String { /// Builds the emergency rollback command. /// /// Restores the most recent OmnySSH backup of sshd_config and reloads the daemon. +/// A missing backup exits 0 so its marker reaches the caller: a checked run drops +/// the output of a command that fails. pub fn build_rollback_command() -> String { r#"BACKUP=$(find /etc/ssh -maxdepth 1 -name 'sshd_config.omnyssh_backup.*' 2>/dev/null | sort | tail -1); \ if [ -n "$BACKUP" ]; then \ sudo cp "$BACKUP" /etc/ssh/sshd_config && \ (sudo systemctl reload sshd 2>/dev/null || sudo systemctl reload ssh 2>/dev/null || sudo service sshd reload 2>/dev/null || sudo service ssh reload); \ else \ - echo "OMNYSSH_NO_BACKUP"; exit 1; \ + echo "OMNYSSH_NO_BACKUP"; \ fi"# .to_string() } @@ -517,11 +552,39 @@ pub async fn setup_key_for_host( key_type: KeyType, progress_tx: Option>, ) -> Result { + match run_key_setup(host, password_session, key_type, progress_tx).await { + (result, None) => Ok(result), + (_, Some(error)) => Err(error), + } +} + +/// Runs the same process as [`setup_key_for_host`], but every outcome comes back +/// as the result: a failed run ends `FailedSafe`, `RolledBack` or `NeedsRollback` +/// (the rollback failed), with `error_message` and `password_may_be_off` set. +pub async fn setup_key_for_host_detailed( + host: &Host, + password_session: &SshSession, + key_type: KeyType, + progress_tx: Option>, +) -> KeySetupResult { + run_key_setup(host, password_session, key_type, progress_tx) + .await + .0 +} + +/// Shared runner: the result, plus the error a failed run ends with. +async fn run_key_setup( + host: &Host, + password_session: &SshSession, + key_type: KeyType, + progress_tx: Option>, +) -> (KeySetupResult, Option) { let mut machine = KeySetupMachine::new(); let mut result = KeySetupResult { key_path: PathBuf::new(), state: KeySetupState::NotStarted, error_message: None, + password_may_be_off: false, }; // The two verification steps reconnect, so they walk the host's ProxyJump @@ -538,30 +601,22 @@ pub async fn setup_key_for_host( key_type, verify_timeout, &mut machine, + &mut result.key_path, progress_tx, ), ) .await { - Ok(Ok(key_path)) => { - result.key_path = key_path; + Ok(Ok(())) => { result.state = machine.state().clone(); - return Ok(result); + return (result, None); } Ok(Err(e)) => { error!("Key setup failed for {}: {}", host.name, e); e } Err(_) => { - // Running out of time is only safe before the point of no return. - // Past it the server has password auth disabled, so the run needs - // the same rollback a failed final check would get. - let step = if machine.password_disabled { - KeySetupStep::FinalCheck - } else { - KeySetupStep::VerifyKeyAuth - }; - machine.step_result(step, Err(anyhow!("timed out"))); + machine.timed_out(); anyhow!( "Key setup timed out after {} seconds", total_timeout.as_secs() @@ -585,19 +640,22 @@ pub async fn setup_key_for_host( result.state = KeySetupState::RolledBack; } } + result.password_may_be_off = machine.password_may_be_off(); - Err(error) + (result, Some(error)) } -/// Internal implementation of the key setup process. +/// Internal implementation of the key setup process. `key_path` is filled as soon +/// as step 1 produced the key, so a failed or timed-out run still reports it. async fn setup_key_internal( host: &Host, password_session: &SshSession, key_type: KeyType, verify_timeout: Duration, machine: &mut KeySetupMachine, + key_path: &mut PathBuf, progress_tx: Option>, -) -> Result { +) -> Result<()> { // Step 1: Generate key pair. info!("Step 1/6: Generating key pair for {}", host.name); if let Some(ref tx) = progress_tx { @@ -606,6 +664,7 @@ async fn setup_key_internal( let (private_key_path, public_key_path) = match generate_key_pair(&host.name, key_type).await { Ok(paths) => { machine.step_result(KeySetupStep::GenerateKey, Ok(())); + *key_path = paths.0.clone(); paths } Err(e) => { @@ -699,7 +758,7 @@ async fn setup_key_internal( warn!("No sudo access — password authentication will NOT be disabled"); machine.set_has_sudo(false); machine.step_result(KeySetupStep::VerifyKeyAuth, Ok(())); // Trigger PartialSuccess. - return Ok(private_key_path); + return Ok(()); } } @@ -709,11 +768,17 @@ async fn setup_key_internal( let _ = tx.send(KeySetupStep::DisablePassword).await; } let disable_cmd = build_disable_password_command(); - match time::timeout(STEP_TIMEOUT, password_session.run_command(&disable_cmd)).await { + machine.disable_sent(); + let disabled = time::timeout(STEP_TIMEOUT, password_session.run_command(&disable_cmd)).await; + // Only the command's own output says what it did to the server. + if matches!(disabled, Ok(Ok(_))) { + machine.disable_answered(); + } + match disabled { Ok(Ok(output)) if output.contains("OMNYSSH_NO_SUDO") => { machine.set_has_sudo(false); machine.step_result(KeySetupStep::VerifyKeyAuth, Ok(())); - return Ok(private_key_path); + return Ok(()); } Ok(Ok(output)) if output.contains("OMNYSSH_CONFIG_ERROR") => { machine.step_result(KeySetupStep::DisablePassword, Err(anyhow!("Config error"))); @@ -770,7 +835,7 @@ async fn setup_key_internal( info!("Final verification passed! Key setup complete."); final_session.disconnect().await; machine.step_result(KeySetupStep::FinalCheck, Ok(())); - Ok(private_key_path) + Ok(()) } Ok(Err(e)) => { error!( @@ -802,16 +867,24 @@ async fn emergency_rollback(session: &SshSession) -> Result<()> { warn!("Attempting emergency rollback of sshd_config"); let rollback_cmd = build_rollback_command(); - match time::timeout(STEP_TIMEOUT, session.run_command(&rollback_cmd)).await { - Ok(Ok(output)) if output.contains("OMNYSSH_NO_BACKUP") => { + // Checked: a failed restore or reload must not count as a rollback. + let output = time::timeout(STEP_TIMEOUT, session.run_command_checked(&rollback_cmd)) + .await + .unwrap_or_else(|_| Err(anyhow!("timeout"))); + rollback_outcome(output) +} + +/// Reads the rollback command's checked run. +fn rollback_outcome(output: Result) -> Result<()> { + match output { + Ok(output) if output.contains("OMNYSSH_NO_BACKUP") => { Err(anyhow!("No backup file found for rollback")) } - Ok(Ok(_)) => { + Ok(_) => { info!("Rollback successful — password authentication restored"); Ok(()) } - Ok(Err(e)) => Err(anyhow!("Rollback failed: {}", e)), - Err(_) => Err(anyhow!("Rollback failed: timeout")), + Err(e) => Err(anyhow!("Rollback failed: {}", e)), } } @@ -822,12 +895,16 @@ async fn emergency_rollback(session: &SshSession) -> Result<()> { /// Result of the key setup process. #[derive(Debug, Clone)] pub struct KeySetupResult { - /// Path to the generated private key file. + /// Path to the generated private key file; set once step 1 produced it, so a + /// failed run reports it too (empty when no key was made). pub key_path: PathBuf, /// Final state of the setup process. pub state: KeySetupState, /// Error message if the setup failed. pub error_message: Option, + /// A failed run may have left the server's password login off: the + /// rollback failed, or the step-4 command's outcome is unknown. + pub password_may_be_off: bool, } // --------------------------------------------------------------------------- @@ -948,6 +1025,97 @@ mod tests { assert!(machine.password_disabled); } + #[test] + fn test_unanswered_disable_may_leave_password_off() { + let mut machine = KeySetupMachine::new(); + machine.step_result(KeySetupStep::VerifyKeyAuth, Ok(())); + machine.disable_sent(); + // Transport error or step timeout: the command never answered. + machine.step_result(KeySetupStep::DisablePassword, Err(anyhow!("timeout"))); + + assert_eq!(machine.state(), &KeySetupState::FailedSafe); + assert!(machine.password_may_be_off()); + } + + #[test] + fn test_total_timeout_during_disable_may_leave_password_off() { + let mut machine = KeySetupMachine::new(); + machine.step_result(KeySetupStep::VerifyKeyAuth, Ok(())); + machine.disable_sent(); + machine.timed_out(); + + // No rollback runs, but the server may already have been changed. + assert_eq!(machine.state(), &KeySetupState::FailedSafe); + assert!(machine.password_may_be_off()); + } + + #[test] + fn test_answered_disable_failure_is_known() { + // OMNYSSH_CONFIG_ERROR: the command restored its backup itself. + let mut machine = KeySetupMachine::new(); + machine.disable_sent(); + machine.disable_answered(); + machine.step_result(KeySetupStep::DisablePassword, Err(anyhow!("config error"))); + + assert_eq!(machine.state(), &KeySetupState::FailedSafe); + assert!(!machine.password_may_be_off()); + } + + #[test] + fn test_failure_before_disable_is_known() { + let mut machine = KeySetupMachine::new(); + machine.step_result(KeySetupStep::VerifyKeyAuth, Err(anyhow!("refused"))); + assert!(!machine.password_may_be_off()); + + let mut machine = KeySetupMachine::new(); + machine.timed_out(); + assert_eq!(machine.state(), &KeySetupState::FailedSafe); + assert!(!machine.password_may_be_off()); + } + + #[test] + fn test_rollback_outcome_decides_password_may_be_off() { + let mut machine = KeySetupMachine::new(); + machine.disable_sent(); + machine.disable_answered(); + machine.step_result(KeySetupStep::DisablePassword, Ok(())); + machine.timed_out(); + + // Past step 4 a timeout needs the rollback; until it succeeds the + // password may be off. + assert_eq!(machine.state(), &KeySetupState::NeedsRollback); + assert!(machine.password_may_be_off()); + + machine.rollback_complete(); + assert!(!machine.password_may_be_off()); + } + + #[test] + fn test_failed_rollback_command_is_a_failed_rollback() { + // A non-zero exit from `sudo cp` or the reload. + let failed = rollback_outcome(Err(anyhow!("remote command exited with status 1"))); + assert!(failed + .unwrap_err() + .to_string() + .starts_with("Rollback failed")); + + let none = rollback_outcome(Ok("OMNYSSH_NO_BACKUP\n".to_string())); + assert_eq!( + none.unwrap_err().to_string(), + "No backup file found for rollback" + ); + + assert!(rollback_outcome(Ok(String::new())).is_ok()); + } + + #[test] + fn test_rollback_command_reports_missing_backup_with_exit_zero() { + // A checked run drops the output of a failing command, marker included. + let cmd = build_rollback_command(); + assert!(cmd.contains("echo \"OMNYSSH_NO_BACKUP\";")); + assert!(!cmd.contains("exit 1")); + } + #[test] fn test_authorized_keys_command_escapes_quotes() { let pubkey = "ssh-ed25519 AAAA... user's key"; diff --git a/crates/omnyssh-gui/capabilities/default.json b/crates/omnyssh-gui/capabilities/default.json index 2cf8c266..9260e47c 100644 --- a/crates/omnyssh-gui/capabilities/default.json +++ b/crates/omnyssh-gui/capabilities/default.json @@ -7,6 +7,9 @@ "core:default", "core:window:allow-set-theme", "core:window:allow-start-dragging", + "core:window:allow-destroy", + "core:window:allow-unminimize", + "core:window:allow-set-focus", "store:default", { "identifier": "opener:allow-open-url", diff --git a/crates/omnyssh-gui/src/bridge.rs b/crates/omnyssh-gui/src/bridge.rs index 73542655..db9f1f91 100644 --- a/crates/omnyssh-gui/src/bridge.rs +++ b/crates/omnyssh-gui/src/bridge.rs @@ -95,25 +95,15 @@ pub async fn forward_core_events( } // Auto key-setup (§4.2): `start_key_setup` drives the core and reports these // on the shared engine channel; the host name identifies the run. - CoreEvent::KeySetupProgress(host_name, step) => { - let _ = events::KeySetupProgress { - host_name, - step: step.into(), - } - .emit(&app); - } - CoreEvent::KeySetupComplete(host_name, key_path) => { - let _ = events::KeySetupComplete { - host_name, - key_path: key_path.to_string_lossy().to_string(), + event @ (CoreEvent::KeySetupProgress(..) + | CoreEvent::KeySetupComplete(..) + | CoreEvent::KeySetupPartial(..) + | CoreEvent::KeySetupFailed(..) + | CoreEvent::KeySetupFailedUnsafe(..) + | CoreEvent::KeySetupRollback(..)) => { + if let Some(outbound) = map_key_setup_event(event) { + outbound.emit(&app); } - .emit(&app); - } - CoreEvent::KeySetupFailed(host_name, error) => { - let _ = events::KeySetupFailed { host_name, error }.emit(&app); - } - CoreEvent::KeySetupRollback(host_name, result) => { - let _ = events::KeySetupRollback { host_name, result }.emit(&app); } // A newer release found by the startup check (§4.3) → the update banner. CoreEvent::UpdateAvailable(info) => { @@ -130,6 +120,61 @@ pub async fn forward_core_events( } } +/// A typed key-setup IPC event, ready to emit. +#[derive(Debug)] +enum KeySetupOutbound { + Progress(events::KeySetupProgress), + Complete(events::KeySetupComplete), + Failed(events::KeySetupFailed), + Rollback(events::KeySetupRollback), +} + +impl KeySetupOutbound { + fn emit(self, app: &AppHandle) { + let _ = match self { + KeySetupOutbound::Progress(e) => e.emit(app), + KeySetupOutbound::Complete(e) => e.emit(app), + KeySetupOutbound::Failed(e) => e.emit(app), + KeySetupOutbound::Rollback(e) => e.emit(app), + }; + } +} + +/// Map a core key-setup event to its IPC event (§4.3): the partial and unsafe +/// variants share the complete / failed events, told apart by their flag. +fn map_key_setup_event(event: CoreEvent) -> Option { + let complete = |host_name, key_path: std::path::PathBuf, password_off| { + KeySetupOutbound::Complete(events::KeySetupComplete { + host_name, + key_path: key_path.to_string_lossy().to_string(), + password_off, + }) + }; + let failed = |host_name, error, password_may_be_off| { + KeySetupOutbound::Failed(events::KeySetupFailed { + host_name, + error, + password_may_be_off, + }) + }; + Some(match event { + CoreEvent::KeySetupProgress(host_name, step) => { + KeySetupOutbound::Progress(events::KeySetupProgress { + host_name, + step: step.into(), + }) + } + CoreEvent::KeySetupComplete(host_name, key_path) => complete(host_name, key_path, true), + CoreEvent::KeySetupPartial(host_name, key_path) => complete(host_name, key_path, false), + CoreEvent::KeySetupFailed(host_name, error) => failed(host_name, error, false), + CoreEvent::KeySetupFailedUnsafe(host_name, error) => failed(host_name, error, true), + CoreEvent::KeySetupRollback(host_name, result) => { + KeySetupOutbound::Rollback(events::KeySetupRollback { host_name, result }) + } + _ => return None, + }) +} + /// Written under a session's last output when it ends: dim, then the cursor hidden /// and any mouse reporting the dead app left on turned off, so a drag selects text. const END_LINE: &[u8] = b"\r\n\x1b[2m[Connection closed. Press Enter to close this tab.]\x1b[0m\ @@ -546,4 +591,44 @@ mod tests { // forwarder robust and the match exhaustive without inventing an event (§3.4). assert!(map_sftp_event(1, CoreEvent::PtyOutput(3)).is_none()); } + + /// The wire payload of a key-setup event as JSON. + fn key_setup_json(event: CoreEvent) -> serde_json::Value { + match map_key_setup_event(event).expect("a key-setup event") { + KeySetupOutbound::Progress(e) => serde_json::to_value(e), + KeySetupOutbound::Complete(e) => serde_json::to_value(e), + KeySetupOutbound::Failed(e) => serde_json::to_value(e), + KeySetupOutbound::Rollback(e) => serde_json::to_value(e), + } + .expect("serialise") + } + + #[test] + fn key_setup_outcomes_carry_the_password_state() { + let key = std::path::PathBuf::from("/k/omnyssh_web_ed25519"); + assert_eq!( + key_setup_json(CoreEvent::KeySetupComplete("web".into(), key.clone())), + serde_json::json!({ + "hostName": "web", "keyPath": "/k/omnyssh_web_ed25519", "passwordOff": true + }) + ); + assert_eq!( + key_setup_json(CoreEvent::KeySetupPartial("web".into(), key))["passwordOff"], + false + ); + assert_eq!( + key_setup_json(CoreEvent::KeySetupFailed("web".into(), "boom".into())), + serde_json::json!({ "hostName": "web", "error": "boom", "passwordMayBeOff": false }) + ); + assert_eq!( + key_setup_json(CoreEvent::KeySetupFailedUnsafe("web".into(), "boom".into())) + ["passwordMayBeOff"], + true + ); + assert_eq!( + key_setup_json(CoreEvent::KeySetupRollback("web".into(), "restored".into())), + serde_json::json!({ "hostName": "web", "result": "restored" }) + ); + assert!(map_key_setup_event(CoreEvent::Error("x".into())).is_none()); + } } diff --git a/crates/omnyssh-gui/src/commands/keysetup.rs b/crates/omnyssh-gui/src/commands/keysetup.rs index 01923f88..a50e3f0d 100644 --- a/crates/omnyssh-gui/src/commands/keysetup.rs +++ b/crates/omnyssh-gui/src/commands/keysetup.rs @@ -1,10 +1,13 @@ //! Auto SSH key-setup command (tech-gui.md §4.2). `start_key_setup` mirrors the TUI's //! orchestration (crates/omnyssh `ConfirmKeySetup`): connect a password session, drive -//! `setup_key_for_host`, and report progress + the terminal outcome as +//! `setup_key_for_host_detailed`, and report progress + the terminal outcome as //! `CoreEvent::KeySetup*` on the shared engine channel — the bridge maps those to -//! `key-setup-*` IPC events (§3.4). On success it writes the generated key back onto the +//! `key-setup-*` IPC events (§3.4). Once the key works it writes it back onto the //! manual host in `hosts.toml`, the same completion write the TUI performs, so a -//! `reload_hosts` afterwards refreshes `hasKey`/`passwordAuthDisabled` on the card. +//! `reload_hosts` afterwards refreshes `hasKey`/`passwordAuthDisabled` on the card. A +//! failure that may have left password login off saves the tested key alone. + +use std::path::Path; use tauri::{AppHandle, Manager, State}; use tokio::sync::mpsc; @@ -12,7 +15,9 @@ use tokio::sync::mpsc; use omnyssh_core::config::{load_hosts, save_hosts}; use omnyssh_core::event::CoreEvent; use omnyssh_core::ssh::client::Host; -use omnyssh_core::ssh::key_setup::{setup_key_for_host, KeySetupState, KeySetupStep, KeyType}; +use omnyssh_core::ssh::key_setup::{ + setup_key_for_host_detailed, KeySetupResult, KeySetupState, KeySetupStep, KeyType, +}; use omnyssh_core::ssh::session::SshSession; use crate::error::CommandError; @@ -52,8 +57,9 @@ impl Drop for KeySetupSlot { } /// The background flow: forward each step as `KeySetupProgress`, connect with the -/// password, run `setup_key_for_host`, then map the final state to exactly one terminal -/// event (`KeySetupComplete` / `KeySetupRollback` / `KeySetupFailed`). +/// password, run `setup_key_for_host_detailed`, then map the final state to exactly one +/// terminal event (`KeySetupComplete` / `KeySetupPartial` / `KeySetupRollback` / +/// `KeySetupFailed` / `KeySetupFailedUnsafe`). async fn run_key_setup(app: AppHandle, host: Host, engine_tx: mpsc::Sender) { // Frees the slot on any return path, including an unwind (see `KeySetupSlot`). let _slot = KeySetupSlot(app); @@ -63,7 +69,7 @@ async fn run_key_setup(app: AppHandle, host: Host, engine_tx: mpsc::Sender(8); let step_tx = engine_tx.clone(); let step_host = host.name.clone(); - tokio::spawn(async move { + let drainer = tokio::spawn(async move { while let Some(step) = progress_rx.recv().await { let _ = step_tx .send(CoreEvent::KeySetupProgress(step_host.clone(), step)) @@ -84,83 +90,161 @@ async fn run_key_setup(app: AppHandle, host: Host, engine_tx: mpsc::Sender - { - let disabled = matches!(result.state, KeySetupState::Success); - persist_key(&host.name, &result.key_path.to_string_lossy(), disabled).await; - let _ = engine_tx - .send(CoreEvent::KeySetupComplete( - host.name.clone(), - result.key_path, - )) - .await; - } - // The two arms below are the correct mapping for a rolled-back / non-terminal - // `Ok`, kept symmetric with the `KeySetupRollback` event the bridge must map - // (§3.4) and mirroring the TUI reference. They are currently unreachable: the - // stable core's `setup_key_for_host` returns `Err` on every rollback/failure path - // and discards the `RolledBack` state (§12), so a rolled-back setup surfaces via - // the `Err` arm as `KeySetupFailed` with the core's message. Distinguishing - // rollback from failure would need the core to surface that outcome — out of scope - // for this slice, not an edit we are authorised to make (§3.6). - Ok(result) if result.state == KeySetupState::RolledBack => { - let reason = result - .error_message - .unwrap_or_else(|| "Rolled back.".to_string()); - let _ = engine_tx - .send(CoreEvent::KeySetupRollback(host.name.clone(), reason)) - .await; - } - Ok(result) => { - let reason = result - .error_message - .unwrap_or_else(|| "Key setup failed.".to_string()); - let _ = engine_tx - .send(CoreEvent::KeySetupFailed(host.name.clone(), reason)) - .await; - } - Err(e) => { - let _ = engine_tx - .send(CoreEvent::KeySetupFailed( - host.name.clone(), - format!("{e:#}"), - )) - .await; + // Persist BEFORE emitting so the frontend's reload sees it. + if let Some(write) = key_write(&result) { + persist_key(&host.name, &result.key_path, write).await; + } + let _ = engine_tx.send(outcome_event(host.name, result)).await; +} + +/// What a finished run writes onto the manual host. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum KeyWrite { + /// The key works: identity, setup date and the password-login state. + Setup { password_disabled: bool }, + /// A failed run that may have turned password login off: only the tested key, so + /// the next connect can still get in. Password and setup state stay as they were. + IdentityOnly, +} + +fn key_write(result: &KeySetupResult) -> Option { + match result.state { + KeySetupState::Success => Some(KeyWrite::Setup { + password_disabled: true, + }), + KeySetupState::PartialSuccess => Some(KeyWrite::Setup { + password_disabled: false, + }), + KeySetupState::RolledBack => None, + // The core only reports may-be-off after the key passed its verify step. + _ if result.password_may_be_off && !result.key_path.as_os_str().is_empty() => { + Some(KeyWrite::IdentityOnly) } + _ => None, } } -/// Write the generated key onto the manual host in `hosts.toml`, mirroring the TUI's -/// completion write: set `identity_file` + `key_setup_date`, and — only when password -/// auth was actually disabled — mark `password_auth_disabled` and drop the stored -/// password (key auth supersedes it). Only manual hosts live in `hosts.toml`, so an -/// SSH-config name is a no-op. Best-effort: the completion event fires regardless, and -/// this is awaited so a following `reload_hosts` observes the write (no read/write race). -async fn persist_key(host_name: &str, key_path: &str, password_disabled: bool) { +/// The one terminal event for a finished run. +fn outcome_event(host_name: String, result: KeySetupResult) -> CoreEvent { + let error = result + .error_message + .unwrap_or_else(|| "Key setup failed.".to_string()); + match result.state { + KeySetupState::Success => CoreEvent::KeySetupComplete(host_name, result.key_path), + KeySetupState::PartialSuccess => CoreEvent::KeySetupPartial(host_name, result.key_path), + KeySetupState::RolledBack => CoreEvent::KeySetupRollback(host_name, error), + _ if result.password_may_be_off => CoreEvent::KeySetupFailedUnsafe(host_name, error), + _ => CoreEvent::KeySetupFailed(host_name, error), + } +} + +/// Write the generated key onto the manual host in `hosts.toml`. `Setup` mirrors the +/// TUI's completion write: set `identity_file` + `key_setup_date` and record whether +/// password auth was disabled; only when it was, drop the stored password (key auth +/// supersedes it, and the server would refuse it anyway). `IdentityOnly` sets just +/// `identity_file`. Only manual hosts live in `hosts.toml`, so an SSH-config name is a +/// no-op. Best-effort: the outcome event fires regardless, and this is awaited so a +/// following `reload_hosts` observes the write (no read/write race). +async fn persist_key(host_name: &str, key_path: &Path, write: KeyWrite) { let name = host_name.to_string(); - let key_path = key_path.to_string(); + let key_path = key_path.to_string_lossy().to_string(); let _ = tauri::async_runtime::spawn_blocking(move || -> Option<()> { let mut hosts = load_hosts().ok()?; let host = hosts.iter_mut().find(|h| h.name == name)?; host.identity_file = Some(key_path); - host.key_setup_date = Some(chrono::Utc::now().to_rfc3339()); - if password_disabled { - host.password_auth_disabled = Some(true); - host.password = None; + if let KeyWrite::Setup { password_disabled } = write { + host.key_setup_date = Some(chrono::Utc::now().to_rfc3339()); + host.password_auth_disabled = Some(password_disabled); + if password_disabled { + host.password = None; + } } save_hosts(&hosts).ok() }) .await; } + +#[cfg(test)] +mod tests { + use super::*; + use std::path::PathBuf; + + fn result(state: KeySetupState, password_may_be_off: bool) -> KeySetupResult { + KeySetupResult { + key_path: PathBuf::from("/k/omnyssh_web_ed25519"), + state, + error_message: Some("boom".to_string()), + password_may_be_off, + } + } + + #[test] + fn each_outcome_maps_to_its_terminal_event() { + let ev = |state, off| outcome_event("web".to_string(), result(state, off)); + assert!(matches!( + ev(KeySetupState::Success, false), + CoreEvent::KeySetupComplete(h, p) if h == "web" && p.ends_with("omnyssh_web_ed25519") + )); + assert!(matches!( + ev(KeySetupState::PartialSuccess, false), + CoreEvent::KeySetupPartial(h, _) if h == "web" + )); + assert!(matches!( + ev(KeySetupState::RolledBack, false), + CoreEvent::KeySetupRollback(_, r) if r == "boom" + )); + assert!(matches!( + ev(KeySetupState::FailedSafe, false), + CoreEvent::KeySetupFailed(_, e) if e == "boom" + )); + // A failed rollback, or a disable step that never answered. + assert!(matches!( + ev(KeySetupState::NeedsRollback, true), + CoreEvent::KeySetupFailedUnsafe(_, e) if e == "boom" + )); + assert!(matches!( + ev(KeySetupState::FailedSafe, true), + CoreEvent::KeySetupFailedUnsafe(..) + )); + } + + #[test] + fn each_outcome_writes_what_it_proved() { + let write = |state, off| key_write(&result(state, off)); + assert_eq!( + write(KeySetupState::Success, false), + Some(KeyWrite::Setup { + password_disabled: true + }) + ); + assert_eq!( + write(KeySetupState::PartialSuccess, false), + Some(KeyWrite::Setup { + password_disabled: false + }) + ); + // May be off: save the tested key so "Open terminal" can use it, nothing else. + assert_eq!( + write(KeySetupState::NeedsRollback, true), + Some(KeyWrite::IdentityOnly) + ); + assert_eq!( + write(KeySetupState::FailedSafe, true), + Some(KeyWrite::IdentityOnly) + ); + assert_eq!(write(KeySetupState::FailedSafe, false), None); + assert_eq!(write(KeySetupState::RolledBack, false), None); + + // No key made: nothing to save. + let mut keyless = result(KeySetupState::FailedSafe, true); + keyless.key_path = PathBuf::new(); + assert_eq!(key_write(&keyless), None); + } +} diff --git a/crates/omnyssh-gui/src/events.rs b/crates/omnyssh-gui/src/events.rs index 8ced3da3..fee7fb3b 100644 --- a/crates/omnyssh-gui/src/events.rs +++ b/crates/omnyssh-gui/src/events.rs @@ -145,26 +145,30 @@ pub struct KeySetupProgress { pub step: KeySetupStepDto, } -/// Key setup finished successfully — key auth is configured (tech-gui.md §4.3). -/// `keyPath` is the generated private-key path (a path, never key material, §3.4). +/// Key setup finished — key auth is configured (tech-gui.md §4.3). `keyPath` is the +/// generated private-key path (a path, never key material, §3.4). `passwordOff` is +/// false when the server's password login stayed on (no passwordless sudo). #[derive(Debug, Clone, Serialize, Deserialize, specta::Type, tauri_specta::Event)] #[serde(rename_all = "camelCase")] pub struct KeySetupComplete { pub host_name: String, pub key_path: String, + pub password_off: bool, } -/// Key setup failed before touching the server's auth config (tech-gui.md §4.3). -/// Password authentication is never disabled unless a key was verified first. +/// Key setup failed (tech-gui.md §4.3). `passwordMayBeOff` is true when OmnySSH could +/// not confirm the server's password login is on: the rollback failed, or the disable +/// step never answered. Otherwise password login was not changed, or was restored. #[derive(Debug, Clone, Serialize, Deserialize, specta::Type, tauri_specta::Event)] #[serde(rename_all = "camelCase")] pub struct KeySetupFailed { pub host_name: String, pub error: String, + pub password_may_be_off: bool, } /// Key setup rolled the server's sshd config back after a late failure (tech-gui.md -/// §4.3). `result` is the human-readable rollback outcome. +/// §4.3). `result` is the failure that triggered the rollback. #[derive(Debug, Clone, Serialize, Deserialize, specta::Type, tauri_specta::Event)] #[serde(rename_all = "camelCase")] pub struct KeySetupRollback { diff --git a/crates/omnyssh-gui/src/main.rs b/crates/omnyssh-gui/src/main.rs index e9ad0c92..674ba116 100644 --- a/crates/omnyssh-gui/src/main.rs +++ b/crates/omnyssh-gui/src/main.rs @@ -11,6 +11,8 @@ mod commands; mod dto; mod error; mod events; +#[cfg(target_os = "macos")] +mod menu; mod state; mod tray; @@ -246,6 +248,11 @@ fn main() { tray::reveal(app) })); } + // Replaces Tauri's default, whose ⌘W and ⌘Q close and quit without asking. + #[cfg(target_os = "macos")] + { + app = app.menu(menu::build); + } app // Persists UI prefs (theme, sidebar collapse, refresh interval) from the // frontend JS API — no bespoke command (tech-gui.md §4.2, §5.1). diff --git a/crates/omnyssh-gui/src/menu.rs b/crates/omnyssh-gui/src/menu.rs new file mode 100644 index 00000000..628e6d85 --- /dev/null +++ b/crates/omnyssh-gui/src/menu.rs @@ -0,0 +1,84 @@ +//! The macOS app menu. Tauri's default binds ⌘W and ⌘Q to `performClose:` and +//! `terminate:`, which close the window and end the app without asking; this one keeps +//! everything else and routes both through the page instead. + +use tauri::menu::{AboutMetadata, Menu, MenuItem, PredefinedMenuItem, Submenu}; +use tauri::{AppHandle, Wry}; + +pub const QUIT_ID: &str = "app-quit"; +pub const CLOSE_WINDOW_ID: &str = "app-close-window"; + +/// No item claims ⌘W, so the page gets it and closes the active tab. +pub fn build(app: &AppHandle) -> tauri::Result> { + let info = app.package_info(); + let about = AboutMetadata { + name: Some(info.name.clone()), + version: Some(info.version.to_string()), + copyright: app.config().bundle.copyright.clone(), + authors: app.config().bundle.publisher.clone().map(|p| vec![p]), + ..Default::default() + }; + let separator = || PredefinedMenuItem::separator(app); + + let app_menu = Submenu::with_items( + app, + info.name.clone(), + true, + &[ + &PredefinedMenuItem::about(app, None, Some(about))?, + &separator()?, + &PredefinedMenuItem::services(app, None)?, + &separator()?, + &PredefinedMenuItem::hide(app, None)?, + &PredefinedMenuItem::hide_others(app, None)?, + &PredefinedMenuItem::show_all(app, None)?, + &separator()?, + &MenuItem::with_id(app, QUIT_ID, "Quit OmnySSH", true, Some("CmdOrCtrl+Q"))?, + ], + )?; + // Closing goes through `CloseRequested`, so the tray or the page's question still apply. + let file = Submenu::with_items( + app, + "File", + true, + &[&MenuItem::with_id( + app, + CLOSE_WINDOW_ID, + "Close Window", + true, + Some("CmdOrCtrl+Shift+W"), + )?], + )?; + // WKWebView's clipboard and undo shortcuts work only through these items. + let edit = Submenu::with_items( + app, + "Edit", + true, + &[ + &PredefinedMenuItem::undo(app, None)?, + &PredefinedMenuItem::redo(app, None)?, + &separator()?, + &PredefinedMenuItem::cut(app, None)?, + &PredefinedMenuItem::copy(app, None)?, + &PredefinedMenuItem::paste(app, None)?, + &PredefinedMenuItem::select_all(app, None)?, + ], + )?; + let view = Submenu::with_items( + app, + "View", + true, + &[&PredefinedMenuItem::fullscreen(app, None)?], + )?; + let window = Submenu::with_items( + app, + "Window", + true, + &[ + &PredefinedMenuItem::minimize(app, None)?, + // "Zoom" on macOS. + &PredefinedMenuItem::maximize(app, None)?, + ], + )?; + Menu::with_items(app, &[&app_menu, &file, &edit, &view, &window]) +} diff --git a/crates/omnyssh-gui/src/tray.rs b/crates/omnyssh-gui/src/tray.rs index a4cbaa6b..e970dfa3 100644 --- a/crates/omnyssh-gui/src/tray.rs +++ b/crates/omnyssh-gui/src/tray.rs @@ -4,7 +4,8 @@ //! a tray it would have nowhere to come back from. use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::{Mutex, PoisonError}; +use std::sync::{mpsc, Mutex, PoisonError}; +use std::time::Duration; use tauri::menu::{Menu, MenuEvent, MenuItem}; use tauri::tray::{MouseButton, MouseButtonState, TrayIconBuilder, TrayIconEvent}; @@ -55,6 +56,13 @@ pub static REVEALED: AtomicBool = AtomicBool::new(false); /// does not send it straight back. static MINIMIZED: AtomicBool = AtomicBool::new(false); +/// Set by the tray's and the app menu's Quit, taken by the close it starts: that close +/// ends the app instead of hiding into the tray. +static QUITTING: AtomicBool = AtomicBool::new(false); + +/// How long a page has to answer before a close it holds up goes through anyway. +const PAGE_ANSWER: Duration = Duration::from_secs(5); + fn behavior() -> Behavior { *BEHAVIOR.lock().unwrap_or_else(PoisonError::into_inner) } @@ -265,11 +273,66 @@ pub fn reveal(app: &AppHandle) { } } +/// The tray's and the app menu's Quit, as the window's own close: a page listening for +/// it asks first when terminals, transfers, tunnels or snippet runs are live, and with +/// no listener the window closes, which ends the app. +fn request_quit(app: &AppHandle) { + // Never `prevent_exit` anywhere: it would stop these exits too. + let Some(window) = app.get_webview_window("main") else { + return app.exit(0); + }; + // Still coming up, so nothing is live yet, and the page takes a hidden window's + // close for one into the tray. + if !REVEALED.load(Ordering::Acquire) { + return app.exit(0); + } + QUITTING.store(true, Ordering::Release); + // Out of the tray or the Dock: the question has to be seen. + reveal(app); + if window.close().is_err() { + QUITTING.store(false, Ordering::Release); + app.exit(0); + } +} + +/// Set by the page while its close handler is registered (`routes/+layout.svelte`). +const CLOSE_GUARD: &str = "window.__omnysshAsksOnClose === true ? 1 : 0"; + +/// Once the page listens for closes Tauri leaves every close to it, so a hung page +/// would hold the window open for good. `destroy` skips `CloseRequested`. +fn close_unless_page_answers(window: &Window) { + let Some(webview) = window.get_webview_window(window.label()) else { + return; + }; + let (answered, answer) = mpsc::channel(); + // Only the page's own flag counts: a dead web process answers every script at once, + // with an empty result, while its stale listener still holds the close. + let _ = webview.eval_with_callback(CLOSE_GUARD, move |result| { + if result == "1" { + let _ = answered.send(()); + } + }); + let window = window.clone(); + // Never on the main thread: the answer arrives there. + std::thread::spawn(move || { + if answer.recv_timeout(PAGE_ANSWER).is_err() { + let _ = window.destroy(); + } + }); +} + pub fn on_menu_event(app: &AppHandle, event: MenuEvent) { match event.id().as_ref() { SHOW_ID => reveal(app), - // Never `prevent_exit` anywhere: it would stop this too. - QUIT_ID => app.exit(0), + QUIT_ID => request_quit(app), + #[cfg(target_os = "macos")] + crate::menu::QUIT_ID => request_quit(app), + #[cfg(target_os = "macos")] + crate::menu::CLOSE_WINDOW_ID => { + if let Some(window) = app.get_webview_window("main") { + let _ = window.close(); + } + } _ => {} } } @@ -277,9 +340,14 @@ pub fn on_menu_event(app: &AppHandle, event: MenuEvent) { pub fn on_window_event(window: &Window, event: &WindowEvent) { let behavior = behavior(); match event { - WindowEvent::CloseRequested { api, .. } if behavior.closes_to_tray() => { - api.prevent_close(); - let _ = window.hide(); + WindowEvent::CloseRequested { api, .. } => { + let quitting = QUITTING.swap(false, Ordering::AcqRel); + if behavior.closes_to_tray() && !quitting { + api.prevent_close(); + let _ = window.hide(); + } else { + close_unless_page_answers(window); + } } // Minimizing arrives as a resize, on GTK with the size unchanged. WindowEvent::Resized(_) => { diff --git a/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts b/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts index febf1d29..02605dad 100644 --- a/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts +++ b/crates/omnyssh-gui/ui/e2e/keysetup.spec.ts @@ -2,20 +2,25 @@ import { expect, test, type Page } from '@playwright/test'; // Auto SSH-key setup (tech-gui.md §4.2). e2e runs against the static SPA with Tauri // absent, so we stub `__TAURI_INTERNALS__` at the boundary (§6.4). `start_key_setup` is -// faked: it streams `key-setup-progress` events and then `key-setup-complete`, flipping -// the host's hasKey/passwordAuthDisabled so the follow-up `reload_hosts` replays a keyed -// host — exactly how the real backend drives the panel and refreshes the card. +// faked per outcome: it streams the `key-setup-progress` steps the real run would reach, +// persists what the backend persists (full success: key + password off; partial: key + +// password on; may-be-off failure: the key alone), then emits the terminal event, so the +// follow-up `reload_hosts` replays +// the host as the real backend would leave it. const HOSTS = [ { name: 'pw-host', hostname: 'pw.example.com', user: 'root', port: 22, tags: [], source: 'manual', hasKey: false, localForwards: [], tunnelAutostart: false, forwardAgent: false } ]; -async function boot(page: Page): Promise { +type Outcome = 'success' | 'partial' | 'unsafe' | 'rollback'; + +async function boot(page: Page, outcome: Outcome = 'success'): Promise { await page.addInitScript( - ({ hosts }) => { + ({ hosts, outcome }) => { let cbid = 0; const listeners: Record = {}; const state: { hosts: Array> } = { hosts: hosts.map((h) => ({ ...h })) }; const win = window as unknown as Record; + win.__startCalls = 0; function fire(event: string, payload: unknown): void { for (const id of listeners[event] ?? []) { @@ -24,6 +29,53 @@ async function boot(page: Page): Promise { } } + const LABELS = [ + 'Generating Ed25519 key pair', + 'Copying public key to server', + 'Verifying key authentication', + 'Disabling password authentication', + 'Reloading SSH service', + 'Final verification' + ]; + // The last step each outcome reaches: partial stops after the verify step (no + // passwordless sudo), the unsafe failure in the disable step, and a rollback + // after the final check. + const LAST: Record = { success: 6, partial: 3, unsafe: 4, rollback: 6 }; + + function run(name: string): void { + const last = LAST[outcome]; + for (let i = 1; i <= last; i++) { + setTimeout( + () => fire('key-setup-progress', { hostName: name, step: { index: i, total: 6, description: LABELS[i - 1] } }), + 40 * i + ); + } + setTimeout(() => { + const h = state.hosts.find((x) => (x as { name: string }).name === name); + const keyPath = `/home/me/.ssh/omnyssh_${name}_ed25519`; + if (outcome === 'success' || outcome === 'partial') { + // The real backend persists before emitting complete. + if (h) { + h.hasKey = true; + h.passwordAuthDisabled = outcome === 'success'; + } + fire('key-setup-complete', { hostName: name, keyPath, passwordOff: outcome === 'success' }); + } else if (outcome === 'unsafe') { + if (h) h.hasKey = true; + fire('key-setup-failed', { + hostName: name, + error: 'Failed to disable password authentication: timeout', + passwordMayBeOff: true + }); + } else { + fire('key-setup-rollback', { + hostName: name, + result: 'Final verification failed after disabling password. Attempting rollback.' + }); + } + }, 40 * last + 200); + } + (win as { __TAURI_INTERNALS__: unknown }).__TAURI_INTERNALS__ = { invoke: (cmd: string, args: Record) => { switch (cmd) { @@ -32,24 +84,10 @@ async function boot(page: Page): Promise { case 'reload_hosts': setTimeout(() => fire('hosts-loaded', [...state.hosts]), 0); return Promise.resolve(null); - case 'start_key_setup': { - const name = args.hostName as string; - const step = (index: number, description: string) => - fire('key-setup-progress', { hostName: name, step: { index, total: 6, description } }); - setTimeout(() => step(1, 'Generating Ed25519 key pair'), 40); - setTimeout(() => step(3, 'Verifying key authentication'), 120); - setTimeout(() => { - // The real backend persists the key before emitting complete; mirror - // that so the panel's reload shows a keyed host. - const h = state.hosts.find((x) => (x as { name: string }).name === name); - if (h) { - h.hasKey = true; - h.passwordAuthDisabled = true; - } - fire('key-setup-complete', { hostName: name, keyPath: `/home/me/.ssh/omnyssh_${name}_ed25519` }); - }, 400); + case 'start_key_setup': + win.__startCalls = (win.__startCalls as number) + 1; + run(args.hostName as string); return Promise.resolve(null); - } case 'plugin:event|listen': { const { event, handler } = args as { event: string; handler: number }; (listeners[event] ||= []).push(handler); @@ -66,33 +104,118 @@ async function boot(page: Page): Promise { } }; }, - { hosts: HOSTS } + { hosts: HOSTS, outcome } ); await page.goto('/'); await expect(page.getByText('pw-host', { exact: true })).toBeVisible(); } -test('prompts key setup, streams progress, then reflects key auth on the card', async ({ page }) => { +const startCalls = (page: Page) => page.evaluate(() => (window as unknown as { __startCalls: number }).__startCalls); + +/** Open the confirm dialog from the card and accept it. */ +async function confirmSetup(page: Page): Promise { + await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + await page.getByRole('dialog', { name: 'Set up key login' }).getByRole('button', { name: 'Set up key login' }).click(); +} + +test('asks before key setup, with Cancel focused, and Cancel starts nothing', async ({ page }) => { await boot(page); - // A password host with no key offers the host-first "Set up key" action (§4.2). await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + const confirm = page.getByRole('dialog', { name: 'Set up key login' }); + await expect(confirm.getByText('Set up key login for pw-host?')).toBeVisible(); + // Desktop Chrome's user agent says Windows, so the path is the Windows one. + await expect(confirm.getByText('%USERPROFILE%\\.ssh\\omnyssh_pw-host_ed25519')).toBeVisible(); + await expect(confirm.getByText('root@pw.example.com')).toBeVisible(); + await expect(confirm.getByText(/for every account and device/)).toBeVisible(); + await expect(confirm.getByRole('button', { name: 'Cancel' })).toBeFocused(); - const dialog = page.getByRole('dialog', { name: 'Key setup' }); - await expect(dialog).toBeVisible(); + await confirm.getByRole('button', { name: 'Cancel' }).click(); + await expect(page.getByRole('dialog')).toHaveCount(0); + expect(await startCalls(page)).toBe(0); + + // Escape cancels too. + await page.getByRole('button', { name: 'Set up an SSH key for pw-host' }).click(); + await expect(confirm).toBeVisible(); + await page.keyboard.press('Escape'); + await expect(page.getByRole('dialog')).toHaveCount(0); + expect(await startCalls(page)).toBe(0); +}); - // A step streams in while the flow runs. - await expect(dialog.getByText('Verifying key authentication')).toBeVisible(); +test('a full success says password login is off and the card shows the shield', async ({ page }) => { + await boot(page, 'success'); + await confirmSetup(page); - // Completion shows the generated key path. - await expect(dialog.getByText('Key authentication configured')).toBeVisible(); + const dialog = page.getByRole('dialog', { name: 'Key setup' }); + await expect(dialog).toBeVisible(); + await expect(dialog.getByText('Key login is set up and password login is off')).toBeVisible(); await expect(dialog.getByText(/omnyssh_pw-host_ed25519/)).toBeVisible(); + await expect(dialog.getByText('The password saved in OmnySSH was removed.')).toBeVisible(); + expect(await startCalls(page)).toBe(1); await dialog.getByRole('button', { name: 'Done' }).click(); await expect(page.getByRole('dialog')).toHaveCount(0); - // The reload flipped hasKey/passwordAuthDisabled: the card now reads key-only and no + // The reload flipped hasKey/passwordAuthDisabled: the card shows the shield and no // longer offers key setup. - await expect(page.getByText('key-only')).toBeVisible(); + await expect(page.getByRole('img', { name: 'Key login only' })).toBeVisible(); await expect(page.getByRole('button', { name: 'Set up an SSH key for pw-host' })).toHaveCount(0); }); + +test('a partial run says password login is still on and offers to turn it off', async ({ page }) => { + await boot(page, 'partial'); + await confirmSetup(page); + + const dialog = page.getByRole('dialog', { name: 'Key setup' }); + await expect(dialog.getByText(/Key login works\. Password login is still on/)).toBeVisible(); + await expect(dialog.getByText('Your saved password was kept.')).toBeVisible(); + await expect(dialog.getByText(/password login is off/)).toHaveCount(0); + await dialog.getByRole('button', { name: 'Done' }).click(); + + const badge = page.getByRole('button', { name: 'Password on' }); + await expect(badge).toBeVisible(); + await expect(page.getByRole('img', { name: 'Key login only' })).toHaveCount(0); + + await badge.click(); + const rerun = page.getByRole('dialog', { name: 'Turn off password login' }); + await expect(rerun.getByText('Turn off password login on pw-host?')).toBeVisible(); + await expect(rerun.getByText(/Reuse the login key OmnySSH made before/)).toBeVisible(); + await expect(rerun.getByRole('button', { name: 'Cancel' })).toBeFocused(); + await rerun.getByRole('button', { name: 'Turn off password login' }).click(); + await expect(page.getByRole('dialog', { name: 'Key setup' })).toBeVisible(); + expect(await startCalls(page)).toBe(2); +}); + +test('a failure that may leave password login off says so', async ({ page }) => { + await boot(page, 'unsafe'); + await confirmSetup(page); + + const dialog = page.getByRole('dialog', { name: 'Key setup' }); + await expect(dialog.getByText('Key setup failed — password login may be off')).toBeVisible(); + await expect(dialog.getByText(/off\s+now, or after the SSH service next restarts/)).toBeVisible(); + await expect(dialog.getByText(/The new key was saved for this\s+host/)).toBeVisible(); + await expect(dialog.getByText('sudo sshd -T | grep -i passwordauthentication')).toBeVisible(); + await expect(dialog.getByText(/from the newest/)).toBeVisible(); + await expect(dialog.getByText('Failed to disable password authentication: timeout')).toBeVisible(); + await expect(dialog.getByText('Password login was not changed.')).toHaveCount(0); + await expect(dialog.getByRole('button', { name: 'Open terminal' })).toBeVisible(); + await expect(dialog.getByRole('button', { name: 'Close' })).toBeVisible(); + + // The reload picked up the saved key, so the card stops offering key setup. + await expect(page.getByTitle('Key authentication configured')).toBeVisible(); + await expect(page.getByRole('button', { name: 'Set up an SSH key for pw-host' })).toHaveCount(0); +}); + +test('a rollback says password login was turned back on', async ({ page }) => { + await boot(page, 'rollback'); + await confirmSetup(page); + + const dialog = page.getByRole('dialog', { name: 'Key setup' }); + await expect(dialog.getByText('Password login was turned back on')).toBeVisible(); + await expect(dialog.getByText(/A step after password login was turned off failed/)).toBeVisible(); + await expect(dialog.getByText(/restored the\s+server's SSH settings from its backup/)).toBeVisible(); + await expect(dialog.getByText(/stopped working/)).toHaveCount(0); + await expect( + dialog.getByText('Cause: Final verification failed after disabling password. Attempting rollback.') + ).toBeVisible(); +}); diff --git a/crates/omnyssh-gui/ui/e2e/quit.spec.ts b/crates/omnyssh-gui/ui/e2e/quit.spec.ts new file mode 100644 index 00000000..8a2910b0 --- /dev/null +++ b/crates/omnyssh-gui/ui/e2e/quit.spec.ts @@ -0,0 +1,243 @@ +import { expect, test, type Page } from '@playwright/test'; + +// Quitting with live work asks first (tech-gui.md §4.3). Tauri is stubbed at +// `__TAURI_INTERNALS__` (§6.4): the window's close, and the tray's or app menu's Quit, +// arrive as `tauri://close-requested`, and the stub records every command so a test can +// tell whether the window was destroyed. +const HOSTS = [ + { name: 'web-1', hostname: 'web-1.example.com', user: 'deploy', port: 22, tags: [], source: 'manual', hasKey: true, localForwards: [], tunnelAutostart: false, forwardAgent: false } +]; + +type Call = { cmd: string; args: Record }; + +async function boot(page: Page, options: { tunnel?: boolean; visible?: boolean } = {}): Promise { + await page.addInitScript( + ({ hosts, tunnel, visible }) => { + let cbid = 0; + const listeners: Record = {}; + const win = window as unknown as Record; + const calls: Call[] = []; + win.__calls = calls; + + function fire(event: string, payload: unknown): void { + for (const id of listeners[event] ?? []) { + const cb = win[`__cb${id}`] as ((e: unknown) => void) | undefined; + cb?.({ event, id, payload }); + } + } + win.__fire = fire; + + (win as { __TAURI_INTERNALS__: unknown }).__TAURI_INTERNALS__ = { + metadata: { + currentWindow: { label: 'main' }, + currentWebview: { windowLabel: 'main', label: 'main' } + }, + invoke: (cmd: string, args: Record) => { + calls.push({ cmd, args }); + switch (cmd) { + case 'list_hosts': + return Promise.resolve(hosts); + case 'reload_hosts': + if (tunnel) { + setTimeout(() => fire('tunnel-status-changed', { hostName: 'web-1', status: { kind: 'up' } }), 0); + } + return Promise.resolve(null); + case 'set_tray_behavior': + return Promise.resolve({ available: true, minimize: true }); + case 'terminal_open': { + const chId = (args.onOutput as { id: number }).id; + setTimeout(() => { + const cb = win[`__cb${chId}`] as ((m: unknown) => void) | undefined; + cb?.({ message: new TextEncoder().encode('ready> ').buffer, index: 0 }); + }, 0); + return Promise.resolve(1); + } + case 'plugin:window|is_visible': + return Promise.resolve(visible); + case 'plugin:event|listen': { + const { event, handler } = args as { event: string; handler: number }; + (listeners[event] ||= []).push(handler); + return Promise.resolve(handler); + } + default: + return Promise.resolve(null); + } + }, + transformCallback: (cb: unknown) => { + const id = ++cbid; + win[`__cb${id}`] = cb; + return id; + }, + unregisterCallback: (id: number) => { + delete win[`__cb${id}`]; + } + }; + }, + { hosts: HOSTS, tunnel: options.tunnel ?? false, visible: options.visible ?? true } + ); + await page.goto('/'); + await expect(page.getByText('web-1', { exact: true })).toBeVisible(); +} + +const calls = (page: Page) => + page.evaluate(() => ((window as unknown as { __calls: Call[] }).__calls).map((c) => c.cmd)); + +const fire = (page: Page, event: string, payload: unknown = null) => + page.evaluate( + ([e, p]) => (window as unknown as { __fire: (e: string, p: unknown) => void }).__fire(e as string, p), + [event, payload] + ); + +const closeWindow = (page: Page) => fire(page, 'tauri://close-requested'); + +async function openTerminal(page: Page): Promise { + await page.getByTitle('sh on web-1').click(); + await expect(page.locator('.xterm-rows')).toContainText('ready>'); +} + +const dialog = (page: Page) => page.getByRole('dialog', { name: 'Quit OmnySSH?' }); + +test('closing with a live terminal and tunnel asks first, and Cancel keeps the app', async ({ page }) => { + await boot(page, { tunnel: true }); + await openTerminal(page); + + await closeWindow(page); + await expect(dialog(page)).toBeVisible(); + await expect(dialog(page).getByText('1 terminal and 1 tunnel will be closed.')).toBeVisible(); + // Not the terminal, which had the keyboard: a stray Enter must not quit. + await expect(dialog(page).getByRole('button', { name: 'Cancel' })).toBeFocused(); + + await page.keyboard.press('Enter'); + await expect(dialog(page)).toHaveCount(0); + expect(await calls(page)).not.toContain('plugin:window|destroy'); + await expect(page.locator('.xterm-rows')).toContainText('ready>'); +}); + +test('Quit destroys the window', async ({ page }) => { + await boot(page, { tunnel: true }); + await closeWindow(page); + await expect(dialog(page).getByText('1 tunnel will be closed.')).toBeVisible(); + await dialog(page).getByRole('button', { name: 'Quit' }).click(); + await expect.poll(() => calls(page)).toContain('plugin:window|destroy'); +}); + +test("Don't ask again sticks, and the next close quits without asking", async ({ page }) => { + await boot(page, { tunnel: true }); + await closeWindow(page); + await dialog(page).getByLabel("Don't ask again").check(); + await dialog(page).getByRole('button', { name: 'Quit' }).click(); + await expect.poll(() => calls(page)).toContain('plugin:window|destroy'); + expect(await page.evaluate(() => localStorage.getItem('omnyssh-confirm-quit'))).toBe('false'); + const stored = await page.evaluate(() => + (window as unknown as { __calls: Call[] }).__calls.find((c) => c.cmd === 'plugin:store|set')?.args + ); + expect(stored).toMatchObject({ key: 'confirmQuit', value: false }); + + // The next launch. + await page.reload(); + await expect(page.getByText('web-1', { exact: true })).toBeVisible(); + await closeWindow(page); + await expect.poll(() => calls(page)).toContain('plugin:window|destroy'); + await expect(dialog(page)).toHaveCount(0); + + // Settings turns the question back on. + await page.getByRole('button', { name: 'Settings' }).click(); + const ask = page.getByRole('switch', { name: 'Ask before quitting' }); + await expect(ask).toHaveAttribute('aria-checked', 'false'); + await ask.click(); + await expect(ask).toHaveAttribute('aria-checked', 'true'); + expect(await page.evaluate(() => localStorage.getItem('omnyssh-confirm-quit'))).toBe('true'); +}); + +test('with nothing live the window closes at once', async ({ page }) => { + await boot(page); + await closeWindow(page); + await expect.poll(() => calls(page)).toContain('plugin:window|destroy'); + await expect(dialog(page)).toHaveCount(0); +}); + +test('a window the tray already hid is left alone', async ({ page }) => { + await boot(page, { tunnel: true, visible: false }); + await closeWindow(page); + await expect.poll(() => calls(page)).toContain('plugin:window|is_visible'); + await page.waitForTimeout(100); + expect(await calls(page)).not.toContain('plugin:window|destroy'); + await expect(dialog(page)).toHaveCount(0); +}); + +test('the question brings a minimized window forward, once', async ({ page }) => { + await boot(page, { tunnel: true }); + await closeWindow(page); + await expect(dialog(page).getByText('1 tunnel will be closed.')).toBeVisible(); + await expect + .poll(() => calls(page)) + .toEqual(expect.arrayContaining(['plugin:window|unminimize', 'plugin:window|set_focus'])); + + // A second close while it is open does not stack another. + await closeWindow(page); + await expect(dialog(page)).toHaveCount(1); + + // Keeping it running in the tray instead goes to the setting. + await dialog(page).getByRole('button', { name: 'Keep running in the tray instead' }).click(); + await expect(dialog(page)).toHaveCount(0); + await expect(page.getByRole('switch', { name: 'Close to tray' })).toBeVisible(); + expect(await calls(page)).not.toContain('plugin:window|destroy'); +}); + +test('with nothing live a minimized window closes where it is', async ({ page }) => { + await boot(page); + await closeWindow(page); + await expect.poll(() => calls(page)).toContain('plugin:window|destroy'); + await expect(dialog(page)).toHaveCount(0); + expect(await calls(page)).not.toContain('plugin:window|unminimize'); +}); + +// The Desktop Chrome device reports a Windows user agent: Ctrl+W is the shell's. +test('off macOS Ctrl+W goes to the terminal', async ({ page }) => { + await boot(page); + await openTerminal(page); + await page.locator('.xterm-helper-textarea').focus(); + await page.keyboard.press('Control+w'); + await expect(page.getByRole('button', { name: 'web-1 · terminal', exact: true })).toBeVisible(); +}); + +test.describe('on macOS', () => { + test.use({ + userAgent: + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko)' + }); + + test('⌘W closes the active tab, not the window', async ({ page }) => { + await boot(page); + await openTerminal(page); + await page.locator('.xterm-helper-textarea').focus(); + await page.keyboard.press('Meta+w'); + await expect(page.getByRole('button', { name: 'web-1 · terminal', exact: true })).toHaveCount(0); + await expect(page.locator('.xterm')).toHaveCount(0); + expect(await calls(page)).not.toContain('plugin:window|destroy'); + + // With no tab left it does nothing. + await page.keyboard.press('Meta+w'); + await expect(page.getByText('web-1', { exact: true })).toBeVisible(); + expect(await calls(page)).not.toContain('plugin:window|destroy'); + }); + + test('⌘W leaves the tab alone while a dialog is open', async ({ page }) => { + await boot(page); + await openTerminal(page); + await closeWindow(page); + await expect(dialog(page)).toBeVisible(); + await page.keyboard.press('Meta+w'); + await expect(page.getByRole('button', { name: 'web-1 · terminal', exact: true })).toBeVisible(); + await expect(dialog(page).getByRole('button', { name: 'Keep running in the menu bar instead' })).toBeVisible(); + }); + + test('⌘W leaves the tab alone while the command palette is open', async ({ page }) => { + await boot(page); + await openTerminal(page); + await page.keyboard.press('Meta+k'); + await expect(page.getByRole('dialog', { name: 'Command palette' })).toBeVisible(); + await page.keyboard.press('Meta+w'); + await expect(page.getByRole('button', { name: 'web-1 · terminal', exact: true })).toBeVisible(); + }); +}); diff --git a/crates/omnyssh-gui/ui/e2e/streamer.spec.ts b/crates/omnyssh-gui/ui/e2e/streamer.spec.ts new file mode 100644 index 00000000..86287947 --- /dev/null +++ b/crates/omnyssh-gui/ui/e2e/streamer.spec.ts @@ -0,0 +1,292 @@ +import { expect, test, type Page } from '@playwright/test'; + +// Streamer mode (tech-gui.md §4.1, 2026-10-09 note). Tauri is stubbed at +// `__TAURI_INTERNALS__` (§6.4); `__fire` plays a core event, so an error carrying a +// real address can be sent the way the backend would send it. +const HOSTS = [ + { + name: 'db', + hostname: 'db.example.com', + user: 'admin', + port: 22, + tags: [], + source: 'manual', + hasKey: true, + monitoring: 'ssh', + localForwards: [{ bindAddress: '192.168.1.5', bindPort: 8080, remoteHost: '10.20.3.7', remotePort: 5432 }], + tunnelAutostart: false, + forwardAgent: false + } +]; + +async function boot(page: Page, streamer: boolean, hosts: unknown[] = HOSTS): Promise { + await page.addInitScript( + ({ hosts, streamer }) => { + localStorage.setItem('omnyssh-streamer-mode', String(streamer)); + let cbid = 0; + const listeners: Record = {}; + const win = window as unknown as Record; + win.__fire = (event: string, payload: unknown): void => { + for (const id of listeners[event] ?? []) { + const cb = win[`__cb${id}`] as ((e: unknown) => void) | undefined; + cb?.({ event, id, payload }); + } + }; + (win as { __TAURI_INTERNALS__: unknown }).__TAURI_INTERNALS__ = { + invoke: (cmd: string, args: Record) => { + switch (cmd) { + case 'list_hosts': + return Promise.resolve(hosts); + case 'save_host': + win.__saved = args; + return Promise.resolve(null); + case 'plugin:event|listen': { + const { event, handler } = args as { event: string; handler: number }; + (listeners[event] ||= []).push(handler); + return Promise.resolve(cbid); + } + default: + return Promise.resolve(null); + } + }, + transformCallback: (cb: unknown) => { + const id = ++cbid; + win[`__cb${id}`] = cb; + return id; + }, + unregisterCallback: (id: number) => { + delete win[`__cb${id}`]; + } + }; + }, + { hosts, streamer } + ); + await page.goto('/'); + await expect(page.getByText('1 host', { exact: true })).toBeVisible(); +} + +const TUNNEL_ERROR = "Tunnel to 'db': 8080:10.20.3.7:5432 could not be opened: db.example.com refused"; + +async function fireError(page: Page, message: string): Promise { + await page.evaluate( + (message) => (window as unknown as { __fire: (e: string, p: unknown) => void }).__fire('error', { message }), + message + ); +} + +test('streamer mode keeps real addresses out of the status bar and its tooltip', async ({ page }) => { + await boot(page, true); + await fireError(page, TUNNEL_ERROR); + + const status = page.getByRole('contentinfo').getByText("Tunnel to 'db'"); + await expect(status).toBeVisible(); + for (const real of ['10.20.3.7', 'db.example.com']) { + await expect(status).not.toContainText(real); + await expect(status).not.toHaveAttribute('title', new RegExp(real.replaceAll('.', '\\.'))); + } + await expect(status).toContainText(':5432 could not be opened'); +}); + +test('with streamer mode off the status bar shows the error as sent', async ({ page }) => { + await boot(page, false); + await fireError(page, TUNNEL_ERROR); + await expect(page.getByRole('contentinfo').getByText(TUNNEL_ERROR)).toHaveAttribute('title', TUNNEL_ERROR); +}); + +test('an eye in the status bar says streamer mode is on, without changing its height', async ({ page }) => { + await boot(page, false); + const statusBar = page.getByRole('contentinfo'); + const eye = statusBar.getByRole('img', { name: 'Streamer mode is on' }); + await expect(eye).toHaveCount(0); + const before = (await statusBar.boundingBox())?.height; + + await page.keyboard.press('Control+Shift+S'); + await expect(eye).toBeVisible(); + await expect(eye).toHaveAttribute('title', /\(Ctrl\+Shift\+S\)$/); + await expect(statusBar).not.toContainText('Streamer mode'); + expect((await statusBar.boundingBox())?.height).toBe(before); +}); + +test('the command palette toggles streamer mode', async ({ page }) => { + await boot(page, false); + const eye = page.getByRole('contentinfo').getByRole('img', { name: 'Streamer mode is on' }); + + await page.keyboard.press('Control+k'); + const palette = page.getByRole('dialog', { name: 'Command palette' }); + await palette.getByRole('textbox').fill('streamer'); + await expect(palette.getByText('Commands')).toBeVisible(); + await expect(palette.getByRole('button', { name: 'Toggle streamer mode Ctrl+Shift+S' })).toBeVisible(); + await page.keyboard.press('Enter'); + await expect(palette).toHaveCount(0); + await expect(eye).toBeVisible(); + + await page.keyboard.press('Control+k'); + await palette.getByRole('button', { name: /Toggle streamer mode/ }).click(); + await expect(palette).toHaveCount(0); + await expect(eye).toHaveCount(0); +}); + +test('Ctrl+Shift+S toggles streamer mode and re-masks what is already on screen', async ({ page }) => { + await boot(page, false); + await fireError(page, TUNNEL_ERROR); + const statusBar = page.getByRole('contentinfo'); + await expect(statusBar).toContainText('10.20.3.7'); + + await page.keyboard.press('Control+Shift+S'); + await expect(statusBar).toContainText("Tunnel to 'db'"); + await expect(statusBar).not.toContainText('10.20.3.7'); + await expect(statusBar.getByRole('img', { name: 'Streamer mode is on' })).toBeVisible(); + + // Typing in a field does not hold it back. + await page.getByRole('button', { name: 'Search hosts' }).click(); + await page.getByRole('textbox', { name: 'Search hosts' }).press('Control+Shift+S'); + await expect(statusBar).toContainText('10.20.3.7'); + await expect(statusBar.getByRole('img', { name: 'Streamer mode is on' })).toHaveCount(0); + await expect(page.getByRole('textbox', { name: 'Search hosts' })).toHaveValue(''); +}); + +test('the host form shows disguised addresses until Reveal, and saves the real ones', async ({ page }) => { + await boot(page, true); + await page.getByRole('button', { name: 'Edit db' }).click(); + let editor = page.getByRole('dialog', { name: 'Edit host' }); + let hostname = editor.getByLabel('Hostname / IP'); + const local = editor.getByLabel('Forward 1 local port'); + const remote = editor.getByLabel('Forward 1 remote host'); + + await expect(hostname).toHaveAttribute('readonly', ''); + await expect(hostname).toHaveValue(/^[a-z]+\d+\.com$/); + await expect(local).toHaveAttribute('readonly', ''); + await expect(local).toHaveValue(/^\d+\.\d+\.\d+\.\d+:8080$/); + await expect(local).not.toHaveValue('192.168.1.5:8080'); + await expect(remote).toHaveAttribute('readonly', ''); + await expect(remote).not.toHaveValue('10.20.3.7'); + // Edit focus skips the disguised field. + await expect(editor.getByLabel('User', { exact: true })).toBeFocused(); + + await editor.getByRole('button', { name: 'Reveal' }).first().click(); + await expect(hostname).toHaveValue('db.example.com'); + await expect(hostname).toBeFocused(); + await expect(hostname).not.toHaveAttribute('readonly'); + await expect(local).toHaveValue('192.168.1.5:8080'); + await expect(remote).toHaveValue('10.20.3.7'); + await expect(editor.getByRole('button', { name: 'Reveal' })).toHaveCount(0); + + // A reveal lasts for this open form only; saving while disguised keeps the real values. + await editor.getByRole('button', { name: 'Cancel' }).click(); + await page.getByRole('button', { name: 'Edit db' }).click(); + editor = page.getByRole('dialog', { name: 'Edit host' }); + hostname = editor.getByLabel('Hostname / IP'); + await expect(hostname).toHaveAttribute('readonly', ''); + await editor.getByRole('button', { name: 'Save' }).click(); + await expect(editor).toHaveCount(0); + const saved = await page.evaluate(() => JSON.stringify((window as unknown as { __saved: unknown }).__saved)); + expect(saved).toContain('"hostname":"db.example.com"'); + expect(saved).toContain('"remoteHost":"10.20.3.7"'); +}); + +test('a new host and new forwards stay editable in streamer mode, and save as typed', async ({ page }) => { + await boot(page, true); + await page.getByRole('button', { name: 'Add host' }).click(); + const editor = page.getByRole('dialog', { name: 'Add host' }); + const hostname = editor.getByLabel('Hostname / IP'); + await expect(hostname).not.toHaveAttribute('readonly'); + await expect(editor.getByRole('button', { name: 'Reveal' })).toHaveCount(0); + + await editor.getByLabel('Name', { exact: true }).fill('cache'); + await hostname.fill('cache.example.com'); + await expect(hostname).toHaveValue('cache.example.com'); + await expect(hostname).not.toHaveAttribute('readonly'); + + await editor.getByRole('button', { name: 'Add forward' }).click(); + await editor.getByLabel('Forward 1 local port').fill('6380'); + await editor.getByLabel('Forward 1 remote host').fill('10.9.8.7'); + await editor.getByLabel('Forward 1 remote port').fill('6379'); + await expect(editor.getByLabel('Forward 1 remote host')).toHaveValue('10.9.8.7'); + await expect(editor.getByRole('button', { name: 'Reveal' })).toHaveCount(0); + + await editor.getByRole('button', { name: 'Add host' }).click(); + await expect(editor).toHaveCount(0); + const saved = await page.evaluate(() => JSON.stringify((window as unknown as { __saved: unknown }).__saved)); + expect(saved).toContain('"hostname":"cache.example.com"'); + expect(saved).toContain('"remoteHost":"10.9.8.7"'); +}); + +test('only the forwards present at open are disguised', async ({ page }) => { + await boot(page, true); + await page.getByRole('button', { name: 'Edit db' }).click(); + const editor = page.getByRole('dialog', { name: 'Edit host' }); + + await editor.getByRole('button', { name: 'Add forward' }).click(); + await expect(editor.getByLabel('Forward 1 remote host')).toHaveAttribute('readonly', ''); + const added = editor.getByLabel('Forward 2 remote host'); + await expect(added).not.toHaveAttribute('readonly'); + await added.fill('10.1.2.3'); + await expect(added).toHaveValue('10.1.2.3'); + await expect(editor.getByRole('button', { name: 'Reveal' })).toHaveCount(2); + + // Removing the disguised row leaves the added one, still editable, and no forward to reveal. + await editor.getByRole('button', { name: 'Remove forward 1' }).click(); + await expect(editor.getByLabel('Forward 1 remote host')).toHaveValue('10.1.2.3'); + await expect(editor.getByLabel('Forward 1 remote host')).not.toHaveAttribute('readonly'); + await expect(editor.getByRole('button', { name: 'Reveal' })).toHaveCount(1); + await expect(editor.getByLabel('Hostname / IP')).toHaveAttribute('readonly', ''); +}); + +test('switching streamer mode on mid-edit never locks the field being typed in', async ({ page }) => { + await boot(page, false); + await page.getByRole('button', { name: 'Edit db' }).click(); + const editor = page.getByRole('dialog', { name: 'Edit host' }); + const hostname = editor.getByLabel('Hostname / IP'); + const remote = editor.getByLabel('Forward 1 remote host'); + + await hostname.fill('db2.example.com'); + await hostname.press('Control+Shift+S'); + await expect(page.getByRole('contentinfo').getByRole('img', { name: 'Streamer mode is on' })).toBeVisible(); + await expect(hostname).not.toHaveAttribute('readonly'); + await expect(hostname).toBeFocused(); + await hostname.press('x'); + await expect(hostname).toHaveValue('db2.example.comx'); + // The untouched forward was there at open, so it is disguised now. + await expect(remote).toHaveAttribute('readonly', ''); + await expect(remote).not.toHaveValue('10.20.3.7'); +}); + +test('turning streamer mode off and on again hides a revealed form again', async ({ page }) => { + await boot(page, true); + await page.getByRole('button', { name: 'Edit db' }).click(); + const editor = page.getByRole('dialog', { name: 'Edit host' }); + const hostname = editor.getByLabel('Hostname / IP'); + const remote = editor.getByLabel('Forward 1 remote host'); + + await editor.getByRole('button', { name: 'Reveal' }).first().click(); + await expect(hostname).toHaveValue('db.example.com'); + await hostname.press('Control+Shift+S'); + await hostname.press('Control+Shift+S'); + await expect(page.getByRole('contentinfo').getByRole('img', { name: 'Streamer mode is on' })).toBeVisible(); + await expect(hostname).toHaveAttribute('readonly', ''); + await expect(hostname).not.toHaveValue('db.example.com'); + await expect(remote).toHaveAttribute('readonly', ''); + await expect(remote).not.toHaveValue('10.20.3.7'); + await expect(editor.getByRole('button', { name: 'Reveal' })).toHaveCount(2); +}); + +test('a field its disguise leaves as is stays editable without a Reveal', async ({ page }) => { + await boot(page, true, [ + { ...HOSTS[0], localForwards: [{ bindPort: 8080, remoteHost: 'localhost', remotePort: 80 }] } + ]); + await page.getByRole('button', { name: 'Edit db' }).click(); + const editor = page.getByRole('dialog', { name: 'Edit host' }); + const local = editor.getByLabel('Forward 1 local port'); + const remote = editor.getByLabel('Forward 1 remote host'); + + await expect(local).toHaveValue('8080'); + await expect(local).not.toHaveAttribute('readonly'); + await expect(remote).toHaveValue('localhost'); + await expect(remote).not.toHaveAttribute('readonly'); + // Only the hostname is held, and changing the port leaves it disguised. + await expect(editor.getByRole('button', { name: 'Reveal' })).toHaveCount(1); + await local.fill('9090'); + await expect(local).toHaveValue('9090'); + await expect(editor.getByLabel('Hostname / IP')).toHaveAttribute('readonly', ''); + await expect(editor.getByLabel('Hostname / IP')).not.toHaveValue('db.example.com'); +}); diff --git a/crates/omnyssh-gui/ui/e2e/terminal.spec.ts b/crates/omnyssh-gui/ui/e2e/terminal.spec.ts index fc49245a..40c8517a 100644 --- a/crates/omnyssh-gui/ui/e2e/terminal.spec.ts +++ b/crates/omnyssh-gui/ui/e2e/terminal.spec.ts @@ -379,6 +379,17 @@ test('Ctrl+Shift+C with nothing selected copies nothing', async ({ page }) => { expect(await writes(page)).toEqual([]); }); +test('Ctrl+Shift+S toggles streamer mode and sends the shell no ^S', async ({ page }) => { + await bootWithClipboard(page); + await page.locator('.xterm-helper-textarea').focus(); + + await page.keyboard.press('Control+Shift+S'); + await expect + .poll(() => page.evaluate(() => localStorage.getItem('omnyssh-streamer-mode'))) + .toBe('true'); + expect(await writes(page)).toEqual([]); +}); + // WebKitGTK under a Russian layout reports keyCode 0 for letter keys, which is also // what a synthetic keydown carries unless told otherwise — so this is the key event // xterm gets there: without the fallback, Ctrl+C would send nothing at all. diff --git a/crates/omnyssh-gui/ui/src/lib/bindings.ts b/crates/omnyssh-gui/ui/src/lib/bindings.ts index 3fab408b..7ec40999 100644 --- a/crates/omnyssh-gui/ui/src/lib/bindings.ts +++ b/crates/omnyssh-gui/ui/src/lib/bindings.ts @@ -572,15 +572,17 @@ export type HostsLoaded = HostDto[] */ export type KeyPassphraseRequired = { hostName: string; keyPath: string } /** - * Key setup finished successfully — key auth is configured (tech-gui.md §4.3). - * `keyPath` is the generated private-key path (a path, never key material, §3.4). + * Key setup finished — key auth is configured (tech-gui.md §4.3). `keyPath` is the + * generated private-key path (a path, never key material, §3.4). `passwordOff` is + * false when the server's password login stayed on (no passwordless sudo). */ -export type KeySetupComplete = { hostName: string; keyPath: string } +export type KeySetupComplete = { hostName: string; keyPath: string; passwordOff: boolean } /** - * Key setup failed before touching the server's auth config (tech-gui.md §4.3). - * Password authentication is never disabled unless a key was verified first. + * Key setup failed (tech-gui.md §4.3). `passwordMayBeOff` is true when OmnySSH could + * not confirm the server's password login is on: the rollback failed, or the disable + * step never answered. Otherwise password login was not changed, or was restored. */ -export type KeySetupFailed = { hostName: string; error: string } +export type KeySetupFailed = { hostName: string; error: string; passwordMayBeOff: boolean } /** * A progress step of an auto key-setup run (tech-gui.md §4.3). Mapped by the shared * engine bridge from `CoreEvent::KeySetupProgress`; the host name identifies the run. @@ -588,7 +590,7 @@ export type KeySetupFailed = { hostName: string; error: string } export type KeySetupProgress = { hostName: string; step: KeySetupStepDto } /** * Key setup rolled the server's sshd config back after a late failure (tech-gui.md - * §4.3). `result` is the human-readable rollback outcome. + * §4.3). `result` is the failure that triggered the rollback. */ export type KeySetupRollback = { hostName: string; result: string } /** diff --git a/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte b/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte index 486500e9..22e17c55 100644 --- a/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte +++ b/crates/omnyssh-gui/ui/src/lib/components/AppShell.svelte @@ -13,7 +13,14 @@ import PasswordPrompt from '$lib/screens/PasswordPrompt.svelte'; import UpdateBanner from './UpdateBanner.svelte'; import { support } from '$lib/stores/support'; - import { sidebarCollapsed, isCollapseChord } from '$lib/stores/ui'; + import { get } from 'svelte/store'; + import { sidebarCollapsed, isCollapseChord, isCloseTabChord, isStreamerChord } from '$lib/stores/ui'; + import { activeEntity } from '$lib/stores/activeEntity'; + import { palette } from '$lib/stores/palette'; + import { dialogs } from '$lib/stores/dialogs'; + import { closeSession } from '$lib/stores/navigation'; + import { isMac } from '$lib/platform'; + import { streamerMode } from '$lib/stores/streamer'; let { children }: { children: Snippet } = $props(); @@ -21,6 +28,16 @@ if (isCollapseChord(e)) { e.preventDefault(); sidebarCollapsed.toggle(); + } else if (isMac && isCloseTabChord(e)) { + // As the tab's own close button; never the window, which has ⌘⇧W. + e.preventDefault(); + // The palette is not a dialog, but it covers the session all the same. + if (get(palette).open) return; + const active = get(activeEntity); + if (get(dialogs).length === 0 && active.kind === 'session') closeSession(active.id); + } else if (isStreamerChord(e)) { + e.preventDefault(); + streamerMode.toggle(); } } diff --git a/crates/omnyssh-gui/ui/src/lib/components/CommandPalette.svelte b/crates/omnyssh-gui/ui/src/lib/components/CommandPalette.svelte index fe5aad46..89ed1b52 100644 --- a/crates/omnyssh-gui/ui/src/lib/components/CommandPalette.svelte +++ b/crates/omnyssh-gui/ui/src/lib/components/CommandPalette.svelte @@ -1,8 +1,9 @@ + + +
+

Quit OmnySSH?

+

{summary} will be closed.

+ + {#if $traySupport.available} + + {/if} +
+ + +
+
+ diff --git a/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte b/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte index cc71a69e..c34a4ade 100644 --- a/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte +++ b/crates/omnyssh-gui/ui/src/lib/components/StatusBar.svelte @@ -4,19 +4,36 @@ // (§4.1); colour lives only in the status dots, per the brandbook. import { lastError } from '$lib/stores/notifications'; import { hostSummary } from '$lib/stores/hostSummary'; - import { StatusDot } from '$lib/theme'; + import { maskText, streamerMode } from '$lib/stores/streamer'; + import { streamerChordLabel } from '$lib/stores/ui'; + import { Icon, StatusDot } from '$lib/theme'; + + const streamerLabel = 'Streamer mode is on';