From ad27a7064a78ffc15cf4de57752305e466cd5d68 Mon Sep 17 00:00:00 2001 From: Fabio Nunes <52166187+LittleGuava@users.noreply.github.com> Date: Wed, 4 Nov 2026 22:47:19 -0300 Subject: [PATCH] fix(security): keep saved passwords in the OS credential store (#910) SSH passwords and key passphrases, git HTTPS credentials, the HTTP proxy password and data source passwords were kept in plaintext JSON files in the app data directory. They now live in the OS credential store via keyring 4 (macOS Keychain, Windows Credential Manager, Secret Service on Linux); each file keeps only an index of keys under a per-directory id. Files from older versions are moved key by key on first use, with a .migrating sidecar so a failed or interrupted move retries without duplicating credentials. Without a credential store (headless Linux) the old file is still read but nothing new is written to disk; the secret lasts for the session. New data sources no longer save their password or SSH tunnel secret unless asked. Also keeps the index path in SecretStore for global data sources and runs their test against the in-memory store. Fixes #740 (credentials part) Co-authored-by: LittleGuava <52166187+LittleGuava@users.noreply.github.com> --- Cargo.lock | 156 +++++- Cargo.toml | 3 + crates/ide-db/Cargo.toml | 1 + crates/ide-db/src/lib.rs | 40 ++ crates/ide-db/src/secrets.rs | 81 +-- crates/ide-host/src/lib.rs | 5 +- crates/ide-host/src/proxy.rs | 70 ++- crates/ide-host/tests/proxy.rs | 3 +- crates/ide-ssh/Cargo.toml | 9 +- crates/ide-ssh/src/git_creds.rs | 1 + crates/ide-ssh/src/secrets.rs | 530 ++++++++++++++++++-- crates/ide-ssh/tests/os_credential_store.rs | 44 ++ src/features/database/SourceDialog.test.tsx | 21 + src/features/database/SourceDialog.tsx | 4 +- 14 files changed, 870 insertions(+), 98 deletions(-) create mode 100644 crates/ide-ssh/tests/os_credential_store.rs create mode 100644 src/features/database/SourceDialog.test.tsx diff --git a/Cargo.lock b/Cargo.lock index 937a39aff..2249a2423 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -123,6 +123,17 @@ version = "1.0.104" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "330a5ed07fa54e4702c9d6c4174f74427fc0ef6e214bbd677ae50a5099946470" +[[package]] +name = "apple-native-keyring-store" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b350bfd03649e07aa05c0a81b3e15934374e585c98204a57e20b9d49f49bb9a" +dependencies = [ + "keyring-core", + "log", + "security-framework", +] + [[package]] name = "arbitrary" version = "1.5.0" @@ -1600,6 +1611,7 @@ dependencies = [ "lock_api", "once_cell", "parking_lot_core", + "serde", ] [[package]] @@ -4585,6 +4597,8 @@ dependencies = [ "dirs 6.0.0", "flate2", "ide-core", + "keyring", + "keyring-core", "parking_lot", "russh", "russh-config", @@ -4595,6 +4609,7 @@ dependencies = [ "tempfile", "tokio", "tracing", + "uuid", ] [[package]] @@ -5216,6 +5231,33 @@ dependencies = [ "serde", ] +[[package]] +name = "keyring" +version = "4.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2270074a3d26bcac93c1dc5d2845eb4c089e8d761ccf6e0ea266a16004640627" +dependencies = [ + "apple-native-keyring-store", + "keyring-core", + "windows-native-keyring-store", + "zbus-secret-service-keyring-store", +] + +[[package]] +name = "keyring-core" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29fbf4b551298c67925f38cf341a1ada797a75f1837bdf8ae1fc8e2ebc6efb59" +dependencies = [ + "chrono", + "dashmap", + "log", + "regex", + "ron", + "serde", + "uuid", +] + [[package]] name = "kqueue" version = "1.2.1" @@ -5679,6 +5721,30 @@ dependencies = [ "unicode-segmentation", ] +[[package]] +name = "num" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" +dependencies = [ + "num-bigint 0.4.8", + "num-complex", + "num-integer", + "num-iter", + "num-rational", + "num-traits", +] + +[[package]] +name = "num-bigint" +version = "0.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c89e69e7e0f03bea5ef08013795c25018e101932225a656383bd384495ecc367" +dependencies = [ + "num-integer", + "num-traits", +] + [[package]] name = "num-bigint" version = "0.5.1" @@ -5691,6 +5757,15 @@ dependencies = [ "rand_core 0.10.1", ] +[[package]] +name = "num-complex" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73f88a1307638156682bada9d7604135552957b7818057dcef22705b4d509495" +dependencies = [ + "num-traits", +] + [[package]] name = "num-conv" version = "0.2.2" @@ -5706,6 +5781,27 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-iter" +version = "0.1.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c92800bd69a1eac91786bcfe9da64a897eb72911b8dc3095decbd07429e8048b" +dependencies = [ + "num-integer", + "num-traits", +] + +[[package]] +name = "num-rational" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" +dependencies = [ + "num-bigint 0.4.8", + "num-integer", + "num-traits", +] + [[package]] name = "num-traits" version = "0.2.19" @@ -7208,6 +7304,19 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "ron" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97b3fe97c18ff24299ea56f802c5693eb34266fb0bc8317f4e101debc8a3ceaf" +dependencies = [ + "bitflags 2.13.2", + "serde", + "serde_derive", + "typeid", + "unicode-ident", +] + [[package]] name = "ropey" version = "1.6.1" @@ -7274,7 +7383,7 @@ dependencies = [ "md5", "ml-kem", "module-lattice", - "num-bigint", + "num-bigint 0.5.1", "p256", "p384", "p521", @@ -7655,6 +7764,25 @@ dependencies = [ "zeroize", ] +[[package]] +name = "secret-service" +version = "5.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5107b24b91445dd2aa449a258a1807b63240942157292354dc5bfdbeb8bc6db8" +dependencies = [ + "aes", + "cbc", + "futures-util", + "getrandom 0.4.3", + "hkdf", + "hybrid-array", + "num", + "once_cell", + "serde", + "sha2 0.11.1", + "zbus", +] + [[package]] name = "security-framework" version = "3.7.0" @@ -9143,7 +9271,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom 0.4.3", "once_cell", "rustix", "windows-sys 0.61.2", @@ -10952,6 +11080,19 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-native-keyring-store" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "063426e76fdec7438d56bb777f67e318a84a25c707b07e575cb8b78e10c028f8" +dependencies = [ + "byteorder", + "keyring-core", + "regex", + "windows-sys 0.61.2", + "zeroize", +] + [[package]] name = "windows-numerics" version = "0.3.1" @@ -11488,6 +11629,17 @@ dependencies = [ "zvariant", ] +[[package]] +name = "zbus-secret-service-keyring-store" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74801d001b9e7729adb4f1825b67b398185fed424749aa3d8bacf70417137d9a" +dependencies = [ + "keyring-core", + "secret-service", + "zbus", +] + [[package]] name = "zbus_macros" version = "5.19.0" diff --git a/Cargo.toml b/Cargo.toml index 6976798e9..f51c48953 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -106,6 +106,9 @@ sqlx = { version = "0.9", default-features = false, features = ["runtime-tokio", tiberius = { version = "0.13", default-features = false, features = ["tds73", "rustls", "sql-browser-tokio"] } bb8 = "0.9" uuid = { version = "1", features = ["v4"] } +# OS credential stores +keyring = "4.2" +keyring-core = "1" # testing insta = { version = "1", features = ["yaml"] } diff --git a/crates/ide-db/Cargo.toml b/crates/ide-db/Cargo.toml index 8f9853930..e72eccf23 100644 --- a/crates/ide-db/Cargo.toml +++ b/crates/ide-db/Cargo.toml @@ -29,3 +29,4 @@ uuid.workspace = true [dev-dependencies] tokio = { workspace = true, features = ["rt-multi-thread", "macros"] } tempfile.workspace = true +ide-ssh = { workspace = true, features = ["testing"] } diff --git a/crates/ide-db/src/lib.rs b/crates/ide-db/src/lib.rs index 27717f282..71545ecbb 100644 --- a/crates/ide-db/src/lib.rs +++ b/crates/ide-db/src/lib.rs @@ -1161,9 +1161,49 @@ impl DbManager { #[cfg(test)] mod tests { use super::*; + use ide_ssh::secrets::testing::files_containing; + use std::path::Path; + + fn manager(dir: &Path) -> DbManager { + ide_ssh::secrets::testing::use_memory_store(); + let data = dir.join("data"); + let ssh = Arc::new(SshManager::new(data.join("ssh-passwords.json"))); + DbManager::new(dir.join("project"), data.join("db-passwords.json"), data.join("db-history.json"), ssh) + } + + #[tokio::test] + async fn saved_passwords_stay_off_disk() { + let tmp = tempfile::tempdir().unwrap(); + manager(tmp.path()).set_password("s1", Some("hunter2-db".into()), true).await.unwrap(); + manager(tmp.path()).set_ssh_secret("s1", Some("hunter2-tunnel".into()), true).await.unwrap(); + assert_eq!(files_containing(tmp.path(), "hunter2-db"), Vec::::new()); + assert_eq!(files_containing(tmp.path(), "hunter2-tunnel"), Vec::::new()); + let reopened = manager(tmp.path()); + assert_eq!(reopened.secret(&DataSource::default(), "s1".into(), true).await.as_deref(), Some("hunter2-db")); + assert_eq!( + reopened.secret(&DataSource::default(), secrets::ssh_key("s1"), true).await.as_deref(), + Some("hunter2-tunnel") + ); + } + + #[tokio::test] + async fn moves_plaintext_passwords_of_older_versions_off_disk() { + let tmp = tempfile::tempdir().unwrap(); + let project = tmp.path().join("project").to_string_lossy().into_owned(); + let legacy = serde_json::json!({ project: { "s1": "old-db-pw" }, "/other": { "s2": "other-pw" } }); + std::fs::create_dir_all(tmp.path().join("data")).unwrap(); + std::fs::write(tmp.path().join("data/db-passwords.json"), legacy.to_string()).unwrap(); + let m = manager(tmp.path()); + assert_eq!(m.secret(&DataSource::default(), "s1".into(), true).await.as_deref(), Some("old-db-pw")); + assert_eq!(files_containing(tmp.path(), "old-db-pw"), Vec::::new()); + assert_eq!(files_containing(tmp.path(), "other-pw"), Vec::::new()); + let other = SecretStore::new(tmp.path().join("data/db-passwords.json"), Path::new("/other")); + assert_eq!(other.get("s2").as_deref(), Some("other-pw")); + } #[tokio::test] async fn global_data_sources_are_shared_by_projects_with_their_passwords() { + ide_ssh::secrets::testing::use_memory_store(); let tmp = tempfile::tempdir().unwrap(); let (app, one, two) = (tmp.path().join("app"), tmp.path().join("one"), tmp.path().join("two")); let manager = |root: &std::path::Path| { diff --git a/crates/ide-db/src/secrets.rs b/crates/ide-db/src/secrets.rs index 43a571eb0..9889232a6 100644 --- a/crates/ide-db/src/secrets.rs +++ b/crates/ide-db/src/secrets.rs @@ -1,20 +1,15 @@ -//! Saved data source passwords, in the app's data directory rather than the OS -//! credential store (no system prompts). One JSON file for all projects, keyed -//! by project root, then credential key; owner-only on Unix. +//! Saved data source passwords, in the OS credential store like the other saved +//! passwords ([`SecretFile`]). One index file for all projects; each credential is +//! keyed by project root, then credential key. -use ide_core::{IdeError, IdeResult}; -use parking_lot::Mutex; +use ide_core::IdeResult; +use ide_ssh::secrets::SecretFile; use std::collections::BTreeMap; use std::path::{Path, PathBuf}; -/// Project root → credential key → secret. -type Contents = BTreeMap>; - -/// Serialises read-modify-write across the projects sharing the file. -static LOCK: Mutex<()> = Mutex::new(()); - pub struct SecretStore { path: PathBuf, + file: SecretFile, project: String, } @@ -23,9 +18,24 @@ pub fn ssh_key(id: &str) -> String { format!("{id}#ssh") } +fn file_key(project: &str, key: &str) -> String { + format!("{project}|{key}") +} + +/// Older versions' file: project root → credential key → secret. +fn legacy(value: serde_json::Value) -> Option> { + let all: BTreeMap> = serde_json::from_value(value).ok()?; + Some( + all.into_iter() + .flat_map(|(project, keys)| keys.into_iter().map(move |(k, s)| (file_key(&project, &k), s))) + .collect(), + ) +} + impl SecretStore { pub fn new(path: PathBuf, root: &Path) -> Self { - Self { path, project: root.to_string_lossy().into_owned() } + let file = SecretFile::with_legacy(path.clone(), legacy); + Self { path, file, project: root.to_string_lossy().into_owned() } } /// The same file, keyed by `scope` instead (global data sources: the app data directory). @@ -34,55 +44,15 @@ impl SecretStore { } pub fn get(&self, key: &str) -> Option { - let _g = LOCK.lock(); - self.read().remove(&self.project)?.remove(key) + self.file.get(&file_key(&self.project, key)) } pub fn set(&self, key: &str, secret: &str) -> IdeResult<()> { - let _g = LOCK.lock(); - let mut all = self.read(); - all.entry(self.project.clone()).or_default().insert(key.to_owned(), secret.to_owned()); - self.write(&all) + self.file.set(&file_key(&self.project, key), secret) } pub fn delete(&self, key: &str) { - let _g = LOCK.lock(); - let mut all = self.read(); - let Some(project) = all.get_mut(&self.project) else { return }; - if project.remove(key).is_none() { - return; - } - if project.is_empty() { - all.remove(&self.project); - } - if let Err(e) = self.write(&all) { - tracing::warn!("saved passwords: {}", e.message); - } - } - - fn read(&self) -> Contents { - match std::fs::read(&self.path) { - Ok(bytes) => serde_json::from_slice(&bytes).unwrap_or_else(|e| { - tracing::warn!("saved passwords: {}: {e}", self.path.display()); - Contents::new() - }), - Err(_) => Contents::new(), - } - } - - fn write(&self, all: &Contents) -> IdeResult<()> { - let err = |e: std::io::Error| IdeError::unavailable(format!("saved passwords: {e}")); - if let Some(dir) = self.path.parent() { - std::fs::create_dir_all(dir).map_err(err)?; - } - let tmp = self.path.with_extension("json.tmp"); - let mut options = std::fs::OpenOptions::new(); - options.write(true).create(true).truncate(true); - #[cfg(unix)] - std::os::unix::fs::OpenOptionsExt::mode(&mut options, 0o600); - let bytes = serde_json::to_vec_pretty(all).map_err(|e| IdeError::internal(e.to_string()))?; - std::io::Write::write_all(&mut options.open(&tmp).map_err(err)?, &bytes).map_err(err)?; - std::fs::rename(&tmp, &self.path).map_err(err) + self.file.delete(&file_key(&self.project, key)) } } @@ -92,6 +62,7 @@ mod tests { #[test] fn keeps_projects_apart_and_drops_empty_ones() { + ide_ssh::secrets::testing::use_memory_store(); let tmp = tempfile::tempdir().unwrap(); let file = tmp.path().join("db-passwords.json"); let a = SecretStore::new(file.clone(), Path::new("/p/a")); diff --git a/crates/ide-host/src/lib.rs b/crates/ide-host/src/lib.rs index b0eded265..eb3d08f67 100644 --- a/crates/ide-host/src/lib.rs +++ b/crates/ide-host/src/lib.rs @@ -25,7 +25,7 @@ use ide_language::{LanguageRegistry, Syntax}; use ide_run::ProcessManager; use ide_terminal::TerminalManager; use ide_workspace::SettingsStore; -use parking_lot::RwLock; +use parking_lot::{Mutex, RwLock}; use std::collections::HashMap; use std::path::PathBuf; use std::sync::{Arc, OnceLock}; @@ -74,6 +74,8 @@ pub struct Host { pub environment: environment::Environment, /// The HTTP proxy's saved password (see `proxy.rs`). proxy_password: ide_ssh::secrets::SecretFile, + /// The proxy password when it could not be saved (no credential store). + proxy_session_password: Mutex>, pub(crate) sink_for: SinkFactory, owner: fn(&str) -> &str, pub runtime: tokio::runtime::Handle, @@ -103,6 +105,7 @@ impl Host { let host = Self { trust: trust::TrustStore::load(&config_dir, &data_dir), proxy_password: ide_ssh::secrets::SecretFile::new(data_dir.join("proxy-password.json")), + proxy_session_password: Mutex::default(), remote: Arc::new(ide_remote::RemoteService::new(ssh.clone(), data_dir.join("remote"))), ssh, environment: environment::Environment::new(project_environment, local_data_dir.clone()), diff --git a/crates/ide-host/src/proxy.rs b/crates/ide-host/src/proxy.rs index 1ce2ed021..6644d250b 100644 --- a/crates/ide-host/src/proxy.rs +++ b/crates/ide-host/src/proxy.rs @@ -1,6 +1,7 @@ //! System Settings › HTTP Proxy on the host: the global settings made current for //! connections and child processes, the password kept in the saved passwords -//! (owner-only `proxy-password.json`) instead of `settings.toml`. +//! (the OS credential store) instead of `settings.toml`, or for the session +//! when there is no credential store. use crate::Host; use ide_core::IdeResult; @@ -12,7 +13,9 @@ impl Host { /// `settings` with the saved proxy password filled in, as the Settings dialog edits it. pub fn with_proxy_password(&self, mut settings: Settings) -> Settings { // A password typed into settings.toml by hand counts too, unless one was saved. - if let Some(password) = self.proxy_password.get(PASSWORD_KEY) { + let session = || self.proxy_session_password.lock().clone(); + // The session copy is the latest write that could not be saved. + if let Some(password) = session().or_else(|| self.proxy_password.get(PASSWORD_KEY)) { settings.proxy.password = password; } settings @@ -40,11 +43,70 @@ impl Host { _ => return Ok(Some(value)), }; if let Some(password) = password { - match password.as_str().filter(|p| !p.is_empty()) { - Some(p) => self.proxy_password.set(PASSWORD_KEY, p)?, + let password = password.as_str().filter(|p| !p.is_empty()); + let mut session = self.proxy_session_password.lock(); + *session = None; + match password { + Some(p) => { + if let Err(e) = self.proxy_password.set(PASSWORD_KEY, p) { + tracing::warn!("the HTTP proxy password is kept for this session only: {}", e.message); + *session = Some(p.to_owned()); + } + } None => self.proxy_password.delete(PASSWORD_KEY), } } Ok((key != "proxy.password").then_some(value)) } } + +#[cfg(test)] +mod tests { + use super::*; + use ide_ssh::secrets::{SecretFile, StoreError, StoreResult, Vault}; + use std::sync::Arc; + + /// A machine without a credential store (a headless Linux host). + struct NoStore; + + impl Vault for NoStore { + fn available(&self) -> bool { + false + } + fn get(&self, _: &str, _: &str) -> StoreResult { + Err(StoreError::NoDefaultStore) + } + fn set(&self, _: &str, _: &str, _: &str) -> StoreResult<()> { + Err(StoreError::NoDefaultStore) + } + fn delete(&self, _: &str, _: &str) -> StoreResult<()> { + Err(StoreError::NoDefaultStore) + } + } + + #[tokio::test] + async fn without_a_credential_store_the_password_lasts_the_session() { + let tmp = tempfile::tempdir().unwrap(); + let mut host = Host::new(crate::HostConfig { + config_dir: tmp.path().join("config"), + data_dir: tmp.path().join("data"), + local_data_dir: tmp.path().join("data"), + languages: Arc::new(ide_language::LanguageRegistry::load(None).unwrap()), + syntax: Arc::default(), + sink_for: Box::new(|_| Arc::new(ide_core::NullSink)), + owner: |w| w, + runtime: tokio::runtime::Handle::current(), + project_environment: false, + }); + let path = tmp.path().join("data/proxy-password.json"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, r#"{"httpProxy": "old"}"#).unwrap(); + host.proxy_password = SecretFile::with_vault(path, |v| serde_json::from_value(v).ok(), Arc::new(NoStore)); + + let left = host.take_proxy_password("proxy.password", serde_json::json!("s3cret")).unwrap(); + assert_eq!(left, None); + assert_eq!(host.with_proxy_password(Settings::default()).proxy.password, "s3cret"); + host.take_proxy_password("proxy.password", serde_json::json!("")).unwrap(); + assert_eq!(host.with_proxy_password(Settings::default()).proxy.password, ""); + } +} diff --git a/crates/ide-host/tests/proxy.rs b/crates/ide-host/tests/proxy.rs index cf74d5dd5..91fbb42d1 100644 --- a/crates/ide-host/tests/proxy.rs +++ b/crates/ide-host/tests/proxy.rs @@ -43,6 +43,7 @@ fn walk(dir: &std::path::Path) -> Vec { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn manual_proxy_applies_and_the_password_is_saved_apart() { + ide_ssh::secrets::testing::use_memory_store(); let tmp = tempfile::tempdir().unwrap(); let host = host(tmp.path()); assert_eq!(proxy().mode, ProxyMode::Auto); @@ -61,7 +62,7 @@ async fn manual_proxy_applies_and_the_password_is_saved_apart() { assert!(!settings_files(tmp.path()).contains("s3cret"), "no password in settings.toml"); let saved = std::fs::read_to_string(tmp.path().join("data/proxy-password.json")).unwrap(); - assert!(saved.contains("s3cret")); + assert!(!saved.contains("s3cret"), "the password is in the credential store, not on disk: {saved}"); assert_eq!(ops::project::settings(&host, "main").proxy.password, "s3cret"); // A whole-table write keeps the password out of the file too; an empty one forgets it. diff --git a/crates/ide-ssh/Cargo.toml b/crates/ide-ssh/Cargo.toml index feb93e126..3d99cdf0d 100644 --- a/crates/ide-ssh/Cargo.toml +++ b/crates/ide-ssh/Cargo.toml @@ -14,15 +14,20 @@ tokio.workspace = true tracing.workspace = true parking_lot.workspace = true dirs.workspace = true +uuid.workspace = true +keyring.workspace = true +keyring-core.workspace = true russh = { version = "0.64", default-features = false, features = ["ring", "rsa", "flate2"] } russh-config = "0.58.0" russh-sftp = "2.4" flate2 = "1" [features] -# `testing`: an in-process SSH / SFTP server for tests of crates built on this one. -testing = [] +# `testing`: an in-process SSH / SFTP server for tests of crates built on this one, +# and an in-memory credential store in place of the OS one. +testing = ["keyring-core/sample"] [dev-dependencies] tokio = { workspace = true, features = ["rt-multi-thread", "macros"] } tempfile.workspace = true +keyring-core = { workspace = true, features = ["sample"] } diff --git a/crates/ide-ssh/src/git_creds.rs b/crates/ide-ssh/src/git_creds.rs index bdc818e13..4e839aa40 100644 --- a/crates/ide-ssh/src/git_creds.rs +++ b/crates/ide-ssh/src/git_creds.rs @@ -216,6 +216,7 @@ mod tests { use super::*; fn store() -> (tempfile::TempDir, GitCredentials) { + crate::secrets::testing::use_memory_store(); let tmp = tempfile::tempdir().unwrap(); let path = tmp.path().join("git-credentials.json"); (tmp, GitCredentials { saved: Arc::new(SecretFile::new(path)), sessions: Arc::default() }) diff --git a/crates/ide-ssh/src/secrets.rs b/crates/ide-ssh/src/secrets.rs index 4cd6a9cb8..7359f1571 100644 --- a/crates/ide-ssh/src/secrets.rs +++ b/crates/ide-ssh/src/secrets.rs @@ -1,61 +1,289 @@ -//! Saved passwords (SSH passwords / key passphrases by configuration id, the HTTP -//! proxy's), in the app's data directory rather than the OS credential store (no -//! system prompts). One JSON object, key → secret; owner-only on Unix. +//! Saved passwords (SSH passwords / key passphrases by configuration id, git HTTPS +//! credentials, the HTTP proxy's) in the OS credential store: Windows Credential +//! Manager, the macOS Keychain, the Secret Service on Linux. The file at `path` only +//! indexes them (an id and the keys, never a secret); owner-only on Unix. +//! +//! Older versions kept the secrets themselves in that file: they are moved to the +//! credential store on first use. Without a credential store (a headless Linux +//! host) such a file is still read and forgetting a secret rewrites it without +//! that one, but no new secret is written to disk: saving fails and callers keep +//! the secret for the session only. use ide_core::{IdeError, IdeResult}; +pub use keyring_core::{Error as StoreError, Result as StoreResult}; use parking_lot::Mutex; -use std::collections::BTreeMap; +use serde::{Deserialize, Serialize}; +use std::collections::{BTreeMap, BTreeSet}; use std::path::PathBuf; +use std::sync::Arc; + +const SERVICE: &str = "ai.l0ng.openidea"; + +/// Serialises read-modify-write of the index files (several stores may share one). +static LOCK: Mutex<()> = Mutex::new(()); + +/// Reads the secrets of an older version's file, by key (`None`: not that format). +pub type Legacy = fn(serde_json::Value) -> Option>; + +/// Where the secrets are kept: the default credential store, or a stand-in in tests. +pub trait Vault: Send + Sync { + fn available(&self) -> bool; + fn get(&self, service: &str, key: &str) -> StoreResult; + fn set(&self, service: &str, key: &str, secret: &str) -> StoreResult<()>; + fn delete(&self, service: &str, key: &str) -> StoreResult<()>; +} + +/// `keyring_core`'s default store: the OS's, unless a test installed another one. +struct DefaultStore; + +impl DefaultStore { + fn entry(&self, service: &str, key: &str) -> StoreResult { + if !self.available() { + return Err(StoreError::NoDefaultStore); + } + keyring_core::Entry::new(service, key) + } +} + +impl Vault for DefaultStore { + fn available(&self) -> bool { + if keyring_core::get_default_store().is_some() { + return true; + } + static WARNED: std::sync::Once = std::sync::Once::new(); + match keyring::Entry::store_status() { + Ok(()) => true, + Err(e) => { + WARNED.call_once(|| tracing::warn!("saved passwords: no credential store: {e}")); + false + } + } + } + + fn get(&self, service: &str, key: &str) -> StoreResult { + self.entry(service, key)?.get_password() + } + + fn set(&self, service: &str, key: &str, secret: &str) -> StoreResult<()> { + self.entry(service, key)?.set_password(secret) + } + + fn delete(&self, service: &str, key: &str) -> StoreResult<()> { + self.entry(service, key)?.delete_credential() + } +} pub struct SecretFile { path: PathBuf, - lock: Mutex<()>, + legacy: Legacy, + vault: Arc, +} + +/// The file's contents. `id` keeps the credentials of different data directories +/// (and of tests) apart in the one OS store. +#[derive(Serialize, Deserialize)] +struct Index { + id: String, + keys: BTreeSet, +} + +enum Contents { + Index(Index), + /// An older version's file that could not be moved, and why. + Plaintext(BTreeMap, String), +} + +fn service(id: &str) -> String { + format!("{SERVICE}/{id}") +} + +fn new_id() -> String { + uuid::Uuid::new_v4().to_string() } impl SecretFile { pub fn new(path: PathBuf) -> Self { - Self { path, lock: Mutex::new(()) } + Self::with_legacy(path, |v| serde_json::from_value(v).ok()) + } + + pub fn with_legacy(path: PathBuf, legacy: Legacy) -> Self { + Self::with_vault(path, legacy, Arc::new(DefaultStore)) + } + + pub fn with_vault(path: PathBuf, legacy: Legacy, vault: Arc) -> Self { + Self { path, legacy, vault } } pub fn get(&self, key: &str) -> Option { - let _g = self.lock.lock(); - self.read().remove(key) + let _g = LOCK.lock(); + match self.load() { + Contents::Index(mut index) if index.keys.contains(key) => match self.vault.get(&service(&index.id), key) { + Ok(secret) => Some(secret), + Err(StoreError::NoEntry) => { + // Removed outside the app: stop listing it. + index.keys.remove(key); + self.write_logged(&index); + None + } + Err(e) => { + tracing::warn!("saved passwords: {key}: {e}"); + None + } + }, + Contents::Index(_) => None, + Contents::Plaintext(mut old, _) => old.remove(key), + } } pub fn set(&self, key: &str, secret: &str) -> IdeResult<()> { - let _g = self.lock.lock(); - let mut all = self.read(); - all.insert(key.to_owned(), secret.to_owned()); - self.write(&all) + let _g = LOCK.lock(); + let mut index = match self.load() { + Contents::Index(index) => index, + Contents::Plaintext(_, why) => return Err(unavailable(&why)), + }; + let service = service(&index.id); + self.vault.set(&service, key, secret).map_err(|e| unavailable(&e.to_string()))?; + if index.keys.insert(key.to_owned()) + && let Err(e) = self.write(&index) + { + let _ = self.vault.delete(&service, key); + return Err(e); + } + Ok(()) } pub fn delete(&self, key: &str) { - let _g = self.lock.lock(); - let mut all = self.read(); - if all.remove(key).is_some() - && let Err(e) = self.write(&all) - { - tracing::warn!("{}", e.message); + let _g = LOCK.lock(); + match self.load() { + Contents::Index(mut index) => { + if !index.keys.contains(key) { + return; + } + match self.vault.delete(&service(&index.id), key) { + Ok(()) | Err(StoreError::NoEntry) => {} + Err(e) => { + // Kept in the index so that the next delete tries again. + tracing::warn!("saved passwords: could not forget {key}: {e}"); + return; + } + } + index.keys.remove(key); + self.write_logged(&index); + } + Contents::Plaintext(mut old, _) => { + if old.remove(key).is_none() { + return; + } + // A move halfway may have copied it already. + if let Some(id) = self.pending_id() + && let Err(e) = self.vault.delete(&service(&id), key) + && !matches!(e, StoreError::NoEntry) + { + tracing::warn!("saved passwords: could not forget {key}: {e}"); + return; + } + self.write_logged(&old); + } } } /// Every stored key, sorted (the secrets themselves stay private). pub fn keys(&self) -> Vec { - let _g = self.lock.lock(); - self.read().into_keys().collect() - } - - fn read(&self) -> BTreeMap { - match std::fs::read(&self.path) { - Ok(bytes) => serde_json::from_slice(&bytes).unwrap_or_else(|e| { - tracing::warn!("saved passwords: {}: {e}", self.path.display()); - BTreeMap::new() - }), - Err(_) => BTreeMap::new(), + let _g = LOCK.lock(); + match self.load() { + Contents::Index(index) => index.keys.into_iter().collect(), + Contents::Plaintext(old, _) => old.into_keys().collect(), } } - fn write(&self, all: &BTreeMap) -> IdeResult<()> { + fn load(&self) -> Contents { + let value = match std::fs::read(&self.path) { + Err(_) => return Contents::Index(Index { id: new_id(), keys: BTreeSet::new() }), + Ok(bytes) => match serde_json::from_slice::(&bytes) { + Ok(v) => v, + Err(e) => return self.set_aside(&e.to_string()), + }, + }; + if let Ok(index) = serde_json::from_value::(value.clone()) { + return Contents::Index(index); + } + match (self.legacy)(value) { + Some(old) => self.migrate(old), + None => self.set_aside("unknown format"), + } + } + + /// An unreadable file is renamed rather than overwritten (it may index + /// credentials, or hold secrets); the store starts over. + fn set_aside(&self, why: &str) -> Contents { + let aside = self.path.with_extension(format!("json.unreadable-{}", new_id())); + tracing::warn!("saved passwords: {}: {why}; moved to {}", self.path.display(), aside.display()); + if let Err(e) = std::fs::rename(&self.path, &aside) { + // Not overwritten either: nothing is saved until it can be moved. + return Contents::Plaintext(BTreeMap::new(), format!("{}: {e}", self.path.display())); + } + Contents::Index(Index { id: new_id(), keys: BTreeSet::new() }) + } + + fn pending_path(&self) -> PathBuf { + self.path.with_extension("json.migrating") + } + + /// The id of a move that has not finished (see [`SecretFile::migrate`]). + fn pending_id(&self) -> Option { + let id = std::fs::read_to_string(self.pending_path()).ok()?; + Some(id.trim().to_owned()).filter(|id| !id.is_empty()) + } + + /// Moves an older version's secrets to the credential store, key by key; a secret + /// the store refuses for good (too long, say) is forgotten, and the user is asked + /// for it again. The old file is replaced only when everything else was moved. + /// Until then the id waits in `.migrating`, so a retry after a failure or a + /// crash overwrites the same credentials instead of copying them again. + fn migrate(&self, old: BTreeMap) -> Contents { + if !self.vault.available() { + return Contents::Plaintext(old, "no credential store".into()); + } + let pending = self.pending_path(); + let id = match self.pending_id() { + Some(id) => id, + None => { + let id = new_id(); + if let Err(e) = std::fs::write(&pending, &id) { + return Contents::Plaintext(old, format!("{}: {e}", pending.display())); + } + id + } + }; + let mut index = Index { id, keys: BTreeSet::new() }; + for (key, secret) in &old { + match self.vault.set(&service(&index.id), key, secret) { + Ok(()) => { + index.keys.insert(key.clone()); + } + Err(e @ (StoreError::TooLong(..) | StoreError::Invalid(..))) => { + tracing::warn!("saved passwords: {key} does not fit the credential store and is forgotten: {e}"); + } + Err(e) => { + tracing::warn!("saved passwords: could not move {key} to the credential store: {e}"); + return Contents::Plaintext(old, e.to_string()); + } + } + } + if let Err(e) = self.write(&index) { + return Contents::Plaintext(old, e.message); + } + let _ = std::fs::remove_file(&pending); + Contents::Index(index) + } + + fn write_logged(&self, contents: &impl Serialize) { + if let Err(e) = self.write(contents) { + tracing::warn!("{}", e.message); + } + } + + fn write(&self, contents: &impl Serialize) -> IdeResult<()> { let err = |e: std::io::Error| IdeError::unavailable(format!("saved passwords: {}: {e}", self.path.display())); if let Some(dir) = self.path.parent() { std::fs::create_dir_all(dir).map_err(err)?; @@ -65,18 +293,258 @@ impl SecretFile { options.write(true).create(true).truncate(true); #[cfg(unix)] std::os::unix::fs::OpenOptionsExt::mode(&mut options, 0o600); - let bytes = serde_json::to_vec_pretty(all).map_err(|e| IdeError::internal(e.to_string()))?; + let bytes = serde_json::to_vec_pretty(contents).map_err(|e| IdeError::internal(e.to_string()))?; std::io::Write::write_all(&mut options.open(&tmp).map_err(err)?, &bytes).map_err(err)?; std::fs::rename(&tmp, &self.path).map_err(err) } } +fn unavailable(why: &str) -> IdeError { + IdeError::unavailable(format!("saved passwords: {why}")) +} + +/// Helpers for tests of crates that save secrets (feature `testing`). +#[cfg(any(test, feature = "testing"))] +pub mod testing { + use std::path::{Path, PathBuf}; + + /// Makes an in-memory credential store the default, so that the test never + /// touches the user's keychain. Call it before the first secret is saved. + pub fn use_memory_store() { + static ONCE: std::sync::Once = std::sync::Once::new(); + ONCE.call_once(|| keyring_core::set_default_store(keyring_core::sample::Store::new().expect("memory store"))); + } + + /// Every file under `dir` whose bytes contain `needle`. + pub fn files_containing(dir: &Path, needle: &str) -> Vec { + let mut found = Vec::new(); + let mut pending = vec![dir.to_path_buf()]; + while let Some(d) = pending.pop() { + for entry in std::fs::read_dir(&d).into_iter().flatten().flatten() { + let path = entry.path(); + if path.is_dir() { + pending.push(path); + } else if std::fs::read(&path).is_ok_and(|b| b.windows(needle.len()).any(|w| w == needle.as_bytes())) { + found.push(path); + } + } + } + found + } +} + #[cfg(test)] mod tests { + use super::testing::{files_containing, use_memory_store}; use super::*; + use std::collections::HashMap; + + /// A credential store that can be absent, or fail on chosen keys. + #[derive(Default)] + struct FakeVault { + absent: bool, + entries: Mutex>, + failing: Mutex StoreError>>, + } + + impl FakeVault { + fn fail(&self, key: &str, error: fn() -> StoreError) { + self.failing.lock().insert(key.to_owned(), error); + } + + fn heal(&self) { + self.failing.lock().clear(); + } + + fn check(&self, key: &str) -> StoreResult<()> { + if self.absent { + return Err(StoreError::NoDefaultStore); + } + self.failing.lock().get(key).map_or(Ok(()), |e| Err(e())) + } + + fn secrets(&self) -> Vec { + let mut all: Vec<_> = self.entries.lock().values().cloned().collect(); + all.sort(); + all + } + } + + impl Vault for FakeVault { + fn available(&self) -> bool { + !self.absent + } + fn get(&self, service: &str, key: &str) -> StoreResult { + self.check(key)?; + self.entries.lock().get(&(service.to_owned(), key.to_owned())).cloned().ok_or(StoreError::NoEntry) + } + fn set(&self, service: &str, key: &str, secret: &str) -> StoreResult<()> { + self.check(key)?; + self.entries.lock().insert((service.to_owned(), key.to_owned()), secret.to_owned()); + Ok(()) + } + fn delete(&self, service: &str, key: &str) -> StoreResult<()> { + self.check(key)?; + self.entries.lock().remove(&(service.to_owned(), key.to_owned())).map(|_| ()).ok_or(StoreError::NoEntry) + } + } + + fn platform() -> StoreError { + StoreError::PlatformFailure("locked".into()) + } + + fn with_fake(path: &std::path::Path, vault: &Arc) -> SecretFile { + SecretFile::with_vault(path.to_path_buf(), |v| serde_json::from_value(v).ok(), vault.clone()) + } + + const OLD: &str = r#"{"https://a.example": "me\nold-a", "https://b.example": "me\nold-b"}"#; + + #[test] + fn saved_secrets_never_reach_disk() { + use_memory_store(); + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("ssh-passwords.json"); + SecretFile::new(path.clone()).set("a", "hunter2-ssh").unwrap(); + assert_eq!(files_containing(tmp.path(), "hunter2-ssh"), Vec::::new()); + let reopened = SecretFile::new(path); + assert_eq!(reopened.get("a").as_deref(), Some("hunter2-ssh")); + assert_eq!(reopened.keys(), ["a"]); + } + + #[test] + fn moves_plaintext_files_of_older_versions_off_disk() { + use_memory_store(); + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("git-credentials.json"); + std::fs::write(&path, r#"{"https://example.com": "me\nold-pw"}"#).unwrap(); + let file = SecretFile::new(path.clone()); + assert_eq!(file.keys(), ["https://example.com"]); + assert_eq!(files_containing(tmp.path(), "old-pw"), Vec::::new()); + assert_eq!(SecretFile::new(path).get("https://example.com").as_deref(), Some("me\nold-pw")); + } + + #[test] + fn a_failed_move_keeps_the_old_file_and_the_retry_converges() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("git-credentials.json"); + std::fs::write(&path, OLD).unwrap(); + let vault = Arc::new(FakeVault::default()); + vault.fail("https://b.example", platform); + + let file = with_fake(&path, &vault); + assert_eq!(file.get("https://a.example").as_deref(), Some("me\nold-a")); + assert_eq!(std::fs::read_to_string(&path).unwrap(), OLD, "the old file is untouched"); + assert!(file.set("https://c.example", "me\nnew").unwrap_err().message.contains("locked")); + + vault.heal(); + assert_eq!(file.keys(), ["https://a.example", "https://b.example"]); + assert_eq!(files_containing(tmp.path(), "old-a"), Vec::::new()); + assert_eq!(vault.secrets(), ["me\nold-a", "me\nold-b"], "the retry overwrote, not copied"); + assert!(!path.with_extension("json.migrating").exists()); + assert_eq!(with_fake(&path, &vault).get("https://b.example").as_deref(), Some("me\nold-b")); + } + + #[test] + fn forgetting_during_a_pending_move_forgets_the_copy_too() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("git-credentials.json"); + std::fs::write(&path, OLD).unwrap(); + let vault = Arc::new(FakeVault::default()); + vault.fail("https://b.example", platform); + + let file = with_fake(&path, &vault); + file.delete("https://a.example"); + vault.heal(); + assert_eq!(file.keys(), ["https://b.example"]); + assert_eq!(vault.secrets(), ["me\nold-b"]); + } + + #[test] + fn a_secret_the_store_refuses_for_good_is_forgotten_and_does_not_block_saving() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("git-credentials.json"); + std::fs::write(&path, OLD).unwrap(); + let vault = Arc::new(FakeVault::default()); + vault.fail("https://b.example", || StoreError::TooLong("password".into(), 2560)); + + let file = with_fake(&path, &vault); + assert_eq!(file.keys(), ["https://a.example"]); + assert_eq!(files_containing(tmp.path(), "old-b"), Vec::::new()); + file.set("https://c.example", "me\nnew").unwrap(); + assert_eq!(file.get("https://c.example").as_deref(), Some("me\nnew")); + } + + #[test] + fn without_a_credential_store_the_old_file_is_read_and_nothing_new_is_saved() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("git-credentials.json"); + std::fs::write(&path, OLD).unwrap(); + let vault = Arc::new(FakeVault { absent: true, ..Default::default() }); + + let file = with_fake(&path, &vault); + assert_eq!(file.get("https://a.example").as_deref(), Some("me\nold-a")); + assert!(file.set("https://c.example", "me\nnew").unwrap_err().message.contains("no credential store")); + assert_eq!(files_containing(tmp.path(), "me\nnew"), Vec::::new()); + assert!(!path.with_extension("json.migrating").exists()); + + file.delete("https://a.example"); + assert_eq!(file.keys(), ["https://b.example"]); + assert_eq!(files_containing(tmp.path(), "old-a"), Vec::::new()); + } + + #[test] + fn forgets_only_what_the_store_forgot() { + let tmp = tempfile::tempdir().unwrap(); + let vault = Arc::new(FakeVault::default()); + let file = with_fake(&tmp.path().join("ssh-passwords.json"), &vault); + file.set("a", "pw").unwrap(); + vault.fail("a", platform); + file.delete("a"); + assert_eq!(file.keys(), ["a"], "still listed, so that forgetting can be retried"); + vault.heal(); + file.delete("a"); + assert_eq!(file.keys(), Vec::::new()); + assert_eq!(vault.secrets(), Vec::::new()); + } + + #[test] + fn a_secret_removed_outside_the_app_is_no_longer_listed() { + let tmp = tempfile::tempdir().unwrap(); + let vault = Arc::new(FakeVault::default()); + let file = with_fake(&tmp.path().join("ssh-passwords.json"), &vault); + file.set("a", "pw").unwrap(); + vault.entries.lock().clear(); + assert_eq!(file.get("a"), None); + assert_eq!(file.keys(), Vec::::new()); + } + + #[test] + fn a_failed_index_write_takes_the_secret_back_out_of_the_store() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("ssh-passwords.json"); + std::fs::create_dir(&path).unwrap(); + let vault = Arc::new(FakeVault::default()); + assert!(with_fake(&path, &vault).set("a", "pw").is_err()); + assert_eq!(vault.secrets(), Vec::::new()); + } + + #[test] + fn an_unreadable_file_is_set_aside_not_overwritten() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("ssh-passwords.json"); + std::fs::write(&path, "{ not json").unwrap(); + let vault = Arc::new(FakeVault::default()); + let file = with_fake(&path, &vault); + assert_eq!(file.keys(), Vec::::new()); + file.set("a", "pw").unwrap(); + std::fs::write(&path, "{ not json either").unwrap(); + assert_eq!(file.keys(), Vec::::new()); + assert_eq!(files_containing(tmp.path(), "{ not json").len(), 2, "both kept, neither overwritten"); + } #[test] fn stores_and_forgets_secrets() { + use_memory_store(); let tmp = tempfile::tempdir().unwrap(); let file = SecretFile::new(tmp.path().join("ssh-passwords.json")); file.set("a", "pw").unwrap(); diff --git a/crates/ide-ssh/tests/os_credential_store.rs b/crates/ide-ssh/tests/os_credential_store.rs new file mode 100644 index 000000000..5382b6d74 --- /dev/null +++ b/crates/ide-ssh/tests/os_credential_store.rs @@ -0,0 +1,44 @@ +//! Saved passwords against the real OS credential store (Windows Credential +//! Manager, macOS Keychain, Secret Service). + +use ide_ssh::secrets::SecretFile; + +/// Forgets every key of the file, also when the test panics. +struct Forget<'a>(&'a SecretFile); + +impl Drop for Forget<'_> { + fn drop(&mut self) { + for key in self.0.keys() { + self.0.delete(&key); + } + } +} + +#[test] +#[ignore = "uses the OS credential store"] +fn saves_moves_and_forgets_in_the_os_store() { + assert!(keyring::Entry::store_status().is_ok(), "no OS credential store"); + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("git-credentials.json"); + std::fs::write(&path, r#"{"https://old.example": "me\nold-os-pw"}"#).unwrap(); + let file = SecretFile::new(path.clone()); + let forget = Forget(&file); + file.set("https://new.example", "me\nnew-os-pw").unwrap(); + let on_disk = std::fs::read_to_string(&path).unwrap(); + assert!(!on_disk.contains("old-os-pw") && !on_disk.contains("new-os-pw"), "{on_disk}"); + + let reopened = SecretFile::new(path); + assert_eq!(reopened.get("https://old.example").as_deref(), Some("me\nold-os-pw")); + assert_eq!(reopened.get("https://new.example").as_deref(), Some("me\nnew-os-pw")); + #[cfg(windows)] + { + let index: serde_json::Value = serde_json::from_str(&on_disk).unwrap(); + let service = format!("ai.l0ng.openidea/{}", index["id"].as_str().unwrap()); + let listed = std::process::Command::new("cmdkey").arg("/list").output().unwrap(); + let listed = String::from_utf8_lossy(&listed.stdout); + assert_eq!(listed.matches(service.as_str()).count(), 2, "{listed}"); + } + drop(forget); + assert_eq!(reopened.keys(), Vec::::new()); + assert_eq!(reopened.get("https://old.example"), None); +} diff --git a/src/features/database/SourceDialog.test.tsx b/src/features/database/SourceDialog.test.tsx new file mode 100644 index 000000000..e2cce12f3 --- /dev/null +++ b/src/features/database/SourceDialog.test.tsx @@ -0,0 +1,21 @@ +import { fireEvent, render, screen, within } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import { SourceDialog } from "./SourceDialog"; + +const savePassword = () => screen.getByRole("checkbox", { name: "Save password" }) as HTMLInputElement; + +describe("new data source", () => { + it("does not save the database password unless asked to", () => { + render(); + expect(savePassword().checked).toBe(false); + }); + + it("does not save the SSH tunnel password unless asked to", () => { + render(); + fireEvent.click(screen.getByRole("tab", { name: "SSH/SSL" })); + fireEvent.click(screen.getByRole("checkbox", { name: "Use SSH tunnel" })); + fireEvent.click(screen.getByRole("button", { name: "SSH authentication type" })); + fireEvent.click(within(screen.getByRole("listbox")).getByText("Password")); + expect(savePassword().checked).toBe(false); + }); +}); diff --git a/src/features/database/SourceDialog.tsx b/src/features/database/SourceDialog.tsx index b50c16399..e42153cbf 100644 --- a/src/features/database/SourceDialog.tsx +++ b/src/features/database/SourceDialog.tsx @@ -110,7 +110,7 @@ const blankSsh = (): DbSsh => ({ user: "", auth: "openSsh", keyFile: "", - saveSecret: true, + saveSecret: false, }); const blankOptions = (): DbOptions => ({ @@ -155,7 +155,7 @@ function blank(): DataSource { user: "", file: null, ssl: "prefer", - savePassword: true, + savePassword: false, readOnly: false, color: null, comment: "",