From aeef661d12443e40d2ca399f3df07577cf661644 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Sat, 24 Oct 2026 00:47:46 +0200 Subject: [PATCH 1/8] feat(connections): remember database passwords in the OS secret store A named PostGIS, MySQL/MariaDB or SQL Server connection can keep its password in the operating system's password store, behind an unchecked "Remember on this computer" opt-in in the new, edit and login forms. Nothing is saved by default, Test never persists, and the browser and CLI stay session-only. The stored value is an envelope bound to the connection's id, kind and cleaned address, so a reused id, an interrupted address edit or an old preference never sends an old password. Rename and group edits keep it; an address or username edit, a removal and Forget delete it first and leave the definition unchanged if that fails. Projects stay address-only: opening one reconnects only a unique exact-address remembered match, offers a chooser when several match, and never newly authorizes ambient credentials. A refused saved password is suppressed for the session without touching the store. connection.forgetPassword forgets a stored password; it is reserved to the user, so plugins cannot run it and the assistant asks each time. Driver errors are redacted of the active password at connect, query and reconnect boundaries, keeping DataError::Login. Closes #755. --- crates/assistant/src/tests.rs | 40 ++ crates/data/src/lib.rs | 77 ++++ crates/data/src/source/duckdb.rs | 88 ++++- crates/data/src/source/mysql.rs | 6 +- crates/data/src/source/postgis.rs | 16 +- crates/data/src/source/sqlserver.rs | 4 +- crates/engine/src/cmd/connection_browse.rs | 54 ++- crates/engine/src/cmd/connection_logins.rs | 110 ++++++ crates/engine/src/cmd/connection_use.rs | 103 +++++ crates/engine/src/cmd/connections.rs | 293 ++++++++++++--- crates/engine/src/cmd/items.rs | 34 +- crates/engine/src/cmd/layer.rs | 23 +- crates/engine/src/cmd/plugins.rs | 1 + crates/engine/src/cmd/project.rs | 49 ++- crates/engine/src/connections.rs | 354 +++++++++++++++++- crates/engine/src/lib.rs | 5 + crates/engine/src/tests.rs | 20 +- .../engine/src/tests/remembered_database.rs | 113 ++++++ .../src/tests/remembered_database_packets.rs | 266 +++++++++++++ crates/ui-egui/src/connectionui.rs | 208 +++++++++- crates/ui-egui/src/dialogs.rs | 5 +- crates/ui-egui/src/lib.rs | 3 + crates/ui-egui/src/panes.rs | 44 ++- crates/ui-egui/src/sourcespane.rs | 22 +- .../ui-egui/tests/remembered_database/mod.rs | 236 ++++++++++++ crates/ui-egui/tests/smoke.rs | 3 + docs/user-guide/data.md | 118 ++++-- 27 files changed, 2084 insertions(+), 211 deletions(-) create mode 100644 crates/engine/src/cmd/connection_logins.rs create mode 100644 crates/engine/src/cmd/connection_use.rs create mode 100644 crates/engine/src/tests/remembered_database.rs create mode 100644 crates/engine/src/tests/remembered_database_packets.rs create mode 100644 crates/ui-egui/tests/remembered_database/mod.rs diff --git a/crates/assistant/src/tests.rs b/crates/assistant/src/tests.rs index fd268ee1c..345df398c 100644 --- a/crates/assistant/src/tests.rs +++ b/crates/assistant/src/tests.rs @@ -216,6 +216,46 @@ fn connection_commands_require_fresh_approval_before_login_or_scripts() { } } +#[test] +fn remembered_database_forget_requires_fresh_direct_and_script_approval() { + use rustgis_engine::data::secrets::{MemorySecrets, SecretStore}; + use std::sync::Arc; + for scripted in [false, true] { + let store = Arc::new(MemorySecrets::new()); + let mut s = Session::new(Project::default()); + s.database_logins.set_store(Some(store.clone())); + s.run( + "connection.new", + &json!({"kind":"postgres","name":"Private database","address":"postgres:host=127.0.0.1 port=1 user=approval"}), + ) + .unwrap(); + s.run("connection.login", &json!({"id":"c1","password":"forgotten-sentinel","remember":true})).unwrap(); + let args = if scripted { + json!({"script":"map.rename {\"name\":\"Approved map\"}\nconnection.forgetPassword {\"id\":\"c1\"}"}) + } else { + json!({"command":"connection.forgetPassword","params":{"id":"c1"}}) + }; + let tool = if scripted { "run" } else { "run_command" }; + let mut a = Agent::new(cfg("openai"), &s); + assert!(matches!(a.ask("forget this database's saved password", &s), Step::Send(_))); + let name = s.map().name.clone(); + assert_eq!(a.receive(a.generation(), ok(call("forget", tool, args.clone())), &mut s), Step::Confirm); + assert!(!a.pending(&s).unwrap().2); + assert!(store.load("connection/c1").unwrap().is_some()); + assert_eq!(s.map().name, name); + a.decide(false, false, &mut s); + assert!(store.load("connection/c1").unwrap().is_some()); + assert_eq!(a.receive(a.generation(), ok(call("allow", tool, args.clone())), &mut s), Step::Confirm); + let sent = a.decide(true, true, &mut s); + assert!(matches!(sent, Step::Send(_))); + assert!(!format!("{sent:?}").contains("forgotten-sentinel")); + assert_eq!(store.load("connection/c1").unwrap(), None); + assert!(!s.connections.find("c1").unwrap().remembered); + assert!(s.database_logins.has_session_password("c1")); + assert_eq!(a.receive(a.generation(), ok(call("again", tool, args)), &mut s), Step::Confirm); + } +} + #[test] fn gate_rules() { let s = sample(); diff --git a/crates/data/src/lib.rs b/crates/data/src/lib.rs index dc5e438d5..401483b8f 100644 --- a/crates/data/src/lib.rs +++ b/crates/data/src/lib.rs @@ -60,6 +60,83 @@ pub enum DataError { pub type Result = std::result::Result; +/// Replaces `secret` in `message` with asterisks; the text itself when it holds none. +fn without_secret(message: String, secret: &str) -> String { + if message.contains(secret) { message.replace(secret, "********") } else { message } +} + +/// Whether an I/O error's message holds `secret`. Only one carrying text from elsewhere (a +/// driver's, a server's) can: the system's own errors are fixed text and are not even formatted, +/// so the common case allocates nothing. +fn io_mentions(e: &std::io::Error, secret: &str) -> bool { + e.get_ref().is_some() && e.to_string().contains(secret) +} + +impl DataError { + /// The error with `secret` (a password the driver sent) taken out of its message, whatever a + /// server or driver echoes, keeping the kind: a refused login stays [`DataError::Login`]. + /// An empty secret leaves the error as it is, and so does text without it (nothing is copied). + pub fn redact_secret(self, secret: &str) -> Self { + if secret.is_empty() { + return self; + } + match self { + DataError::Format(m) => DataError::Format(without_secret(m, secret)), + DataError::Login(m) => DataError::Login(without_secret(m, secret)), + DataError::Unsupported(m) => DataError::Unsupported(without_secret(m, secret)), + DataError::NoChunks(m) => DataError::NoChunks(without_secret(m, secret)), + DataError::Io(e) if io_mentions(&e, secret) => { + DataError::Io(std::io::Error::new(e.kind(), without_secret(e.to_string(), secret))) + } + DataError::Io(e) => DataError::Io(e), + } + } +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod secret_redaction_tests { + use super::*; + + #[test] + fn remembered_database_redaction_preserves_classification_and_owned_text() { + let message = String::from("no credential here"); + let pointer = message.as_ptr(); + let DataError::Login(unchanged) = DataError::Login(message).redact_secret("sentinel") else { panic!("lost Login") }; + assert_eq!(unchanged.as_ptr(), pointer); + let e = DataError::Login("server echoed sentinel twice sentinel".into()).redact_secret("sentinel"); + assert!(matches!(e, DataError::Login(_))); + assert_eq!(e.to_string(), "server echoed ******** twice ********"); + let e = DataError::Format("sentinel".into()).redact_secret("sentinel"); + assert!(matches!(e, DataError::Format(_))); + assert_eq!(DataError::Login("unchanged".into()).redact_secret("").to_string(), "unchanged"); + } + + #[derive(Debug)] + struct Pieces; + impl std::fmt::Display for Pieces { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("error: se")?; + f.write_str("nti")?; + f.write_str("nel echoed") + } + } + impl std::error::Error for Pieces {} + + #[test] + fn remembered_database_redaction_handles_fragmented_io_and_preserves_kind() { + let e = DataError::Io(std::io::Error::new(std::io::ErrorKind::PermissionDenied, Pieces)).redact_secret("sentinel"); + let DataError::Io(e) = e else { panic!("lost Io") }; + assert_eq!(e.kind(), std::io::ErrorKind::PermissionDenied); + assert_eq!(e.to_string(), "error: ******** echoed"); + let long = "x".repeat(300); + let e = DataError::Io(std::io::Error::other(format!("before {long} after"))).redact_secret(&long); + assert_eq!(e.to_string(), "i/o: before ******** after"); + let e = DataError::Io(std::io::Error::other("unchanged")).redact_secret(&long); + assert_eq!(e.to_string(), "i/o: unchanged"); + } +} + /// One row: an optional geometry and one value per field. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Feature { diff --git a/crates/data/src/source/duckdb.rs b/crates/data/src/source/duckdb.rs index 7812c43d9..ba8bf5f7d 100644 --- a/crates/data/src/source/duckdb.rs +++ b/crates/data/src/source/duckdb.rs @@ -15,6 +15,7 @@ use super::Family; use super::external::{self, TempFile, fnv1a, output, stamp, touch}; +use crate::secrets::Secret; use crate::{DataError, Result}; use std::path::{Path, PathBuf}; use std::process::Command; @@ -128,25 +129,68 @@ fn split_password(dsn: &str) -> (String, Option) { pub use super::pgconn::public_label; -fn passwords() -> &'static std::sync::Mutex> { - static P: std::sync::OnceLock>> = std::sync::OnceLock::new(); - P.get_or_init(Default::default) +/// A password remembered for a connection's address. `connection_id` names the saved connection +/// it was activated for (see [`set_connection_password`]); `None` is one typed for the address. +struct Entry { + password: Secret, + connection_id: Option, } -/// Remembers a connection's password for this session (never saved: projects name connections -/// without them, and libpq's `PGPASSWORD` or `~/.pgpass` serve when it isn't remembered). +type Passwords = std::collections::HashMap; + +static PASSWORDS: std::sync::LazyLock> = std::sync::LazyLock::new(Default::default); + +fn lock() -> std::sync::MutexGuard<'static, Passwords> { + PASSWORDS.lock().unwrap_or_else(|e| e.into_inner()) +} + +/// Remembers a connection's password for this session, as typed for its address (the saved +/// connections' own are activated by [`set_connection_password`]): projects name connections +/// without them, and libpq's `PGPASSWORD` or `~/.pgpass` serve when it isn't remembered. pub fn remember_password(dsn: &str, password: &str) { - passwords().lock().unwrap_or_else(|e| e.into_inner()).insert(dsn.to_string(), password.to_string()); + lock().insert(dsn.to_string(), Entry { password: Secret::new(password), connection_id: None }); +} + +/// Makes saved connection `id`'s password the one connections to `dsn` use (`Some`), replacing +/// whatever was remembered for the address; or takes away a password a saved connection +/// activated there, whichever one (`None`), leaving one typed for the address. +pub fn set_connection_password(dsn: &str, id: &str, password: Option<&str>) { + let mut all = lock(); + match password { + Some(p) => { + all.insert(dsn.to_string(), Entry { password: Secret::new(p), connection_id: Some(id.to_string()) }); + } + None => { + if all.get(dsn).is_some_and(|e| e.connection_id.is_some()) { + all.remove(dsn); + } + } + } +} + +/// Takes away every password saved connection `id` activated. +pub fn forget_connection_password(id: &str) { + lock().retain(|_, e| e.connection_id.as_deref() != Some(id)); +} + +/// Takes away every password a saved connection activated, leaving the typed ones. +pub fn forget_connection_passwords() { + lock().retain(|_, e| e.connection_id.is_none()); } /// The password remembered for a connection this session, if any. pub(crate) fn remembered_password(dsn: &str) -> Option { - passwords().lock().unwrap_or_else(|e| e.into_inner()).get(dsn).cloned() + lock().get(dsn).map(|e| e.password.expose().to_string()) } -/// Whether a password is remembered for a connection this session. +/// Whether a password is remembered for a connection this session, typed or activated. pub fn has_password(dsn: &str) -> bool { - passwords().lock().unwrap_or_else(|e| e.into_inner()).contains_key(dsn) + lock().contains_key(dsn) +} + +/// Whether a password typed for this address (not one a saved connection activated) is remembered. +pub fn has_explicit_password(dsn: &str) -> bool { + lock().get(dsn).is_some_and(|e| e.connection_id.is_none()) } /// The remembered password of a query's database, handed to DuckDB's PostgreSQL client in its @@ -154,7 +198,7 @@ pub fn has_password(dsn: &str) -> bool { /// would show it. fn password(database: Option<&Database>) -> Option { match database? { - Database::Postgres(dsn) => passwords().lock().unwrap_or_else(|e| e.into_inner()).get(dsn).cloned(), + Database::Postgres(dsn) => remembered_password(dsn), Database::File(_) | Database::Mysql(_) | Database::Sqlserver(_) => None, } } @@ -798,6 +842,30 @@ fn ours(name: &str) -> bool { mod tests { use super::*; + #[test] + fn remembered_database_cache_ownership_and_explicit_preservation() { + let a = "host=remembered_cache_a user=one"; + let b = "mysql://remembered_cache_b/db"; + set_connection_password(a, "cache-c1", Some("one")); + assert!(!has_explicit_password(a)); + assert_eq!(remembered_password(a).as_deref(), Some("one")); + set_connection_password(a, "cache-c2", Some("two")); + forget_connection_password("cache-c1"); + assert_eq!(remembered_password(a).as_deref(), Some("two")); + set_connection_password(a, "cache-c1", None); + assert!(!has_password(a)); + remember_password(a, "explicit"); + set_connection_password(a, "cache-c2", None); + assert!(has_explicit_password(a)); + assert_eq!(remembered_password(a).as_deref(), Some("explicit")); + set_connection_password(b, "cache-c2", Some("owned")); + forget_connection_passwords(); + assert!(!has_password(b)); + assert_eq!(remembered_password(a).as_deref(), Some("explicit")); + assert_eq!(password(Some(&Database::Postgres(a.into()))).as_deref(), Some("explicit")); + lock().remove(a); + } + #[test] fn databases_by_name() { // Quoted passwords with spaces, spaced `=`, escapes; an `@` past a URI's authority. diff --git a/crates/data/src/source/mysql.rs b/crates/data/src/source/mysql.rs index 128cb4492..d9755ae01 100644 --- a/crates/data/src/source/mysql.rs +++ b/crates/data/src/source/mysql.rs @@ -270,7 +270,7 @@ impl Conn { if broken(&e) { *guard = None; } - return Err(err(e)); + return Err(err(e).redact_secret(self.password.as_deref().unwrap_or(""))); } } } @@ -282,6 +282,10 @@ impl Conn { /// offers none, said in the second value), and sets the session read only with a statement /// timeout. fn open_client(config: &Config, password: Option<&str>) -> Result<((mysql::Conn, Server), bool)> { + connect_client(config, password).map_err(|e| e.redact_secret(password.unwrap_or(""))) +} + +fn connect_client(config: &Config, password: Option<&str>) -> Result<((mysql::Conn, Server), bool)> { super::tls_provider(); let base = mysql::OptsBuilder::new() .ip_or_hostname(Some(config.host.clone())) diff --git a/crates/data/src/source/postgis.rs b/crates/data/src/source/postgis.rs index 3e4da55ea..bd20c0784 100644 --- a/crates/data/src/source/postgis.rs +++ b/crates/data/src/source/postgis.rs @@ -168,11 +168,11 @@ impl Conn { let client = guard.as_mut().ok_or_else(|| DataError::Format("not connected".into()))?; let r = f(client); if !client.is_closed() { - return r.map_err(err); + return r.map_err(|e| redacted(&self.config, err(e))); } *guard = None; if attempt == 1 || r.is_ok() { - return r.map_err(err); + return r.map_err(|e| redacted(&self.config, err(e))); } } Err(DataError::Format("not connected".into())) @@ -197,7 +197,19 @@ impl Conn { } } +/// Connects, with the password taken out of any error: a server or the client may echo what it was given. fn open_client(config: &postgres::Config, tls: &Tls) -> Result { + connect_client(config, tls).map_err(|e| redacted(config, e)) +} + +fn redacted(config: &postgres::Config, error: DataError) -> DataError { + match config.get_password().and_then(|p| std::str::from_utf8(p).ok()) { + Some(password) => error.redact_secret(password), + None => error, + } +} + +fn connect_client(config: &postgres::Config, tls: &Tls) -> Result { let mut client = match tls { Tls::Disable => config.connect(postgres::NoTls), Tls::Unverified | Tls::Verified(_) => config.connect(tokio_postgres_rustls::MakeRustlsConnect::new(rustls_config(tls)?)), diff --git a/crates/data/src/source/sqlserver.rs b/crates/data/src/source/sqlserver.rs index 5fa4bc3fb..4a427ef32 100644 --- a/crates/data/src/source/sqlserver.rs +++ b/crates/data/src/source/sqlserver.rs @@ -260,7 +260,7 @@ fn open_on(rt: &tokio::runtime::Runtime, config: &Config, password: Option<&str> .await }) .map_err(|_| DataError::Format(format!("couldn't connect within {} s", config.connect_timeout.as_secs())))? - .map_err(err) + .map_err(|e| err(e).redact_secret(password.unwrap_or(""))) } fn connect_blocking(config: &Config, password: Option<&str>) -> Result<(tokio::runtime::Runtime, Client, u32, String, u32)> { @@ -300,7 +300,7 @@ impl Conn { if broken(&e) { *guard = None; } - return Err(err(e)); + return Err(err(e).redact_secret(self.password.as_deref().unwrap_or(""))); } } } diff --git a/crates/engine/src/cmd/connection_browse.rs b/crates/engine/src/cmd/connection_browse.rs index 9d49dc822..55f9dc581 100644 --- a/crates/engine/src/cmd/connection_browse.rs +++ b/crates/engine/src/cmd/connection_browse.rs @@ -191,7 +191,7 @@ pub(super) fn browse(s: &mut Session, p: &Value) -> CmdResult { return Ok(out); } let found = match &level { - Level::Database => database(&c)?, + Level::Database => database(s, &c)?, Level::Service { url, kind } => service(s, p, &mut out, url, kind.as_deref())?, Level::S3Root if url.is_none() => { buckets(c.options.get("buckets").map(String::as_str).unwrap_or("").split(',').map(str::to_string)) @@ -227,38 +227,30 @@ pub(super) fn browse(s: &mut Session, p: &Value) -> CmdResult { } /// A live database's geometry tables, under their schemas. -fn database(c: &Connection) -> Result { - #[cfg(target_arch = "wasm32")] - { - let _ = c; - Err(DESKTOP_ONLY.into()) - } - #[cfg(not(target_arch = "wasm32"))] - { - use std::collections::BTreeMap; +fn database(s: &mut Session, c: &Connection) -> Result { + use std::collections::BTreeMap; - let tables = super::super::layer::database_table_details(&json!({"database": c.address}))?; - let text = |t: &Value, k: &str| t.get(k).and_then(Value::as_str).unwrap_or("").to_string(); - let mut schemas: BTreeMap> = BTreeMap::new(); - let truncated = tables.len() > MAX_CHILDREN; - for t in tables.iter().take(MAX_CHILDREN) { - let (label, name) = (text(t, "label"), text(t, "name")); - let shown = if label.contains(':') { format!("{name}:{}", text(t, "column")) } else { name }; - schemas.entry(text(t, "schema")).or_default().push(json!({ - "key": label, - "name": shown, - "what": "table", - "detail": text(t, "kind"), - "add": {"command": "layer.addDatabase", "params": {"database": c.address, "layer": label}}, - "options": true, - })); - } - let children = schemas - .into_iter() - .map(|(s, tables)| json!({"key": format!("schema:{s}"), "name": s, "what": "schema", "children": tables})) - .collect(); - Ok(Found { children, truncated, matched: None }) + let tables = super::connection_use::tables(s, c)?; + let text = |t: &Value, k: &str| t.get(k).and_then(Value::as_str).unwrap_or("").to_string(); + let mut schemas: BTreeMap> = BTreeMap::new(); + let truncated = tables.len() > MAX_CHILDREN; + for t in tables.iter().take(MAX_CHILDREN) { + let (label, name) = (text(t, "label"), text(t, "name")); + let shown = if label.contains(':') { format!("{name}:{}", text(t, "column")) } else { name }; + schemas.entry(text(t, "schema")).or_default().push(json!({ + "key": label, + "name": shown, + "what": "table", + "detail": text(t, "kind"), + "add": {"command": "layer.addDatabase", "params": {"database": c.address, "layer": label}}, + "options": true, + })); } + let children = schemas + .into_iter() + .map(|(s, tables)| json!({"key": format!("schema:{s}"), "name": s, "what": "schema", "children": tables})) + .collect(); + Ok(Found { children, truncated, matched: None }) } /// A service's layers (capabilities read here, or the app's `answer` and `documents`). diff --git a/crates/engine/src/cmd/connection_logins.rs b/crates/engine/src/cmd/connection_logins.rs new file mode 100644 index 000000000..97c84287c --- /dev/null +++ b/crates/engine/src/cmd/connection_logins.rs @@ -0,0 +1,110 @@ +//! Resolves address-only project origins against trusted saved definitions, never project IDs. +use crate::Session; +#[cfg(not(target_arch = "wasm32"))] +use crate::connections::{Connection, LoginReason, LoginRequest, database_key, layer_database}; + +pub(crate) fn clear_active() { + #[cfg(not(target_arch = "wasm32"))] + rustgis_data::source::duckdb::forget_connection_passwords(); +} + +pub(crate) fn restore_active(s: &mut Session) { + #[cfg(not(target_arch = "wasm32"))] + for (key, id) in s.database_logins.activations() { + if let Some(pw) = s.database_logins.known_password(&id) { + rustgis_data::source::duckdb::set_connection_password(&key, &id, Some(pw.expose())); + } + } + #[cfg(target_arch = "wasm32")] + let _ = s; +} + +#[cfg(not(target_arch = "wasm32"))] +fn groups(s: &Session) -> std::collections::BTreeMap<(&'static str, String), Vec> { + let mut groups = std::collections::BTreeMap::<_, Vec<_>>::new(); + for l in s.project.maps.iter().flat_map(|m| &m.layers) { + let Some(src) = l.source.as_ref().filter(|src| !src.is_open()) else { continue }; + let Some((_, db)) = layer_database(src) else { continue }; + let Some(key) = database_key(&crate::sources::redact(db)) else { continue }; + groups.entry(key).or_default().push(l.id); + } + groups +} + +#[cfg(not(target_arch = "wasm32"))] +pub(super) fn request_password(s: &mut Session, c: &Connection) { + if let Some((kind, key)) = database_key(&c.address) { + let layers = groups(s).remove(&(kind, key.clone())).unwrap_or_default(); + s.database_logins.request(LoginRequest { + reason: LoginReason::Password, + kind, + key, + connections: vec![c.id.clone()], + layers, + opened: s.opened, + prompted: false, + }); + } +} + +pub(crate) fn resolve_project(s: &mut Session) { + #[cfg(target_arch = "wasm32")] + let _ = s; + #[cfg(not(target_arch = "wasm32"))] + for ((kind, key), layers) in groups(s) { + use rustgis_data::source::duckdb; + if duckdb::has_explicit_password(&key) { + s.database_logins.clear_requests_for(kind, &key); + continue; + } + let selected = s + .database_logins + .selection(kind, &key) + .and_then(|id| s.connections.find(id)) + .filter(|c| database_key(&c.address).is_some_and(|k| k == (kind, key.clone()))) + .cloned(); + let matches: Vec<_> = s.connections.matching_key(kind, &key).into_iter().cloned().collect(); + let candidates: Vec<_> = matches.iter().filter(|c| c.remembered && !s.database_logins.is_rejected(&c.id)).cloned().collect(); + let chosen = if let Some(c) = selected { + Some(c) + } else if candidates.len() == 1 { + candidates.first().cloned() + } else { + duckdb::set_connection_password(&key, "", None); + s.database_logins.drop_activation(&key); + if candidates.len() > 1 { + let ids = candidates.iter().map(|c| c.id.clone()).collect(); + s.database_logins.request(LoginRequest { + reason: LoginReason::Choose, + kind, + key: key.clone(), + connections: ids, + layers: layers.clone(), + opened: s.opened, + prompted: false, + }); + } else if let Some(c) = matches.iter().find(|c| s.database_logins.is_rejected(&c.id)) { + request_password(s, c); + } + None + }; + let Some(c) = chosen else { continue }; + match super::credential(s, &c) { + Ok(Some((pw, saved))) => { + super::activate(s, &c, pw.expose()); + super::reopen(s, &c, saved, true); + } + Ok(None) => { + duckdb::set_connection_password(&key, &c.id, None); + s.database_logins.drop_activation(&key); + request_password(s, &c); + } + Err(e) => { + duckdb::set_connection_password(&key, &c.id, None); + s.database_logins.drop_activation(&key); + s.notify(e, true); + request_password(s, &c); + } + } + } +} diff --git a/crates/engine/src/cmd/connection_use.rs b/crates/engine/src/cmd/connection_use.rs new file mode 100644 index 000000000..af4eded1b --- /dev/null +++ b/crates/engine/src/cmd/connection_use.rs @@ -0,0 +1,103 @@ +//! User-initiated database reads use the requested saved ID, never a same-address row's login. + +use serde_json::Value; +#[cfg(not(target_arch = "wasm32"))] +use serde_json::json; + +use crate::Session; +use crate::connections::Connection; + +/// Geometry-table details shared by saved-ID Browse and Test. Call only after the probe return: +/// a probe must neither read the secret store nor connect to a database. +pub(super) fn tables(s: &mut Session, c: &Connection) -> Result, String> { + #[cfg(target_arch = "wasm32")] + { + let _ = (s, c); + Err(crate::connections::DESKTOP_ONLY.into()) + } + #[cfg(not(target_arch = "wasm32"))] + { + use rustgis_data::DataError; + use rustgis_data::source::duckdb::{self, Database}; + + let database = Database::parse(&c.address); + let (kind, key) = match &database { + Database::Postgres(key) => ("postgres", key), + Database::Mysql(key) => ("mysql", key), + Database::Sqlserver(key) => ("sqlserver", key), + Database::File(_) => return Err("not a database server".into()), + }; + // Clear even when loading fails: another saved ID's active password is not authority + // to read this connection. Passwords explicitly typed for the address remain intact. + duckdb::set_connection_password(key, &c.id, None); + s.database_logins.drop_activation(key); + let credential = if !s.database_logins.has_session_password(&c.id) && duckdb::has_explicit_password(key) { + None + } else { + super::credential(s, c)? + }; + if credential.is_none() && s.database_logins.is_rejected(&c.id) && !duckdb::has_explicit_password(key) { + super::request_password(s, c); + return Err("password is required".into()); + } + let saved = credential.as_ref().is_some_and(|(_, saved)| *saved); + if let Some((password, _)) = &credential { + super::activate(s, c, password.expose()); + s.database_logins.select(kind, key, &c.id); + } + + match read_tables(&database) { + Ok(tables) => { + s.database_logins.clear_requests_for(kind, key); + Ok(tables) + } + Err(error) => { + let error = match &credential { + Some((password, _)) => error.redact_secret(password.expose()), + None => error, + }; + if matches!(&error, DataError::Login(_)) { + if saved { + s.database_logins.reject(&c.id); + duckdb::forget_connection_password(&c.id); + s.database_logins.drop_activation(key); + } + super::request_password(s, c); + } + Err(format!("{}: {error}", database.short())) + } + } + } +} + +#[cfg(not(target_arch = "wasm32"))] +fn read_tables(database: &rustgis_data::source::duckdb::Database) -> rustgis_data::Result> { + use rustgis_data::source::{duckdb::Database, mysql, postgis, sqlserver}; + + // Keep DataError intact through both connect and table listing so only an authentication + // refusal suppresses a saved credential. A transport or query error must not do that. + match database { + Database::Postgres(address) => { + let connection = postgis::connect(address, true)?; + Ok(postgis::tables(&connection)? + .into_iter() + .map(|t| json!({"label": t.label(), "schema": t.schema, "name": t.name, "column": t.column, "kind": t.kind})) + .collect()) + } + Database::Mysql(address) => { + let connection = mysql::connect(address, true)?; + Ok(mysql::tables(&connection)? + .into_iter() + .map(|t| json!({"label": t.label(), "schema": t.schema, "name": t.name, "column": t.column, "kind": t.kind})) + .collect()) + } + Database::Sqlserver(address) => { + let connection = sqlserver::connect(address, true)?; + Ok(sqlserver::tables(&connection)? + .into_iter() + .map(|t| json!({"label": t.label(), "schema": t.schema, "name": t.name, "column": t.column, "kind": t.kind})) + .collect()) + } + Database::File(_) => Err(rustgis_data::DataError::Format("not a database server".into())), + } +} diff --git a/crates/engine/src/cmd/connections.rs b/crates/engine/src/cmd/connections.rs index 904d1da86..30a721268 100644 --- a/crates/engine/src/cmd/connections.rs +++ b/crates/engine/src/cmd/connections.rs @@ -13,6 +13,14 @@ use crate::{CmdResult, CommandSpec as C, Session}; #[path = "connection_browse.rs"] mod browse; +#[path = "connection_logins.rs"] +mod logins; +#[cfg(not(target_arch = "wasm32"))] +use logins::request_password; +pub(crate) use logins::{clear_active, resolve_project, restore_active}; +#[path = "connection_use.rs"] +mod connection_use; + pub fn commands() -> Vec { vec![ C::new("connection.list", "List Connections", "Home › Add › Data Sources", list).view().params( @@ -27,8 +35,9 @@ pub fn commands() -> Vec { C::new("connection.duplicate", "Duplicate Connection", "Home › Add › Data Sources", duplicate).view().params(r#"{"id": "a connection's id or name", "name"?: text}"#), C::new("connection.remove", "Remove Connection", "Home › Add › Data Sources", remove).view().params(r#"{"id": "a connection's id or name"}"#), C::new("connection.login", "Log In to Connection", "Home › Add › Data Sources", login_command).view().params( - r#"{"id": "a connection's id or name", "password"?: "a database connection's, kept for this session only; the project's layers that read its database and couldn't be read open again", "accessKeyId"?: text, "secretAccessKey"?: text, "sessionToken"?: "an S3 connection's keys (credentials: keys), kept for this session only"}"#, + r#"{"id": "a connection's id or name; without password explicitly selects its existing session or remembered database password", "password"?: "database password, session-only by default", "remember"?: "boolean, default false: desktop OS-store opt-in for this password; S3 keys remain session-only", "accessKeyId"?: text, "secretAccessKey"?: text, "sessionToken"?: text}"#, ), + C::new("connection.forgetPassword", "Forget Saved Password", "Home › Add › Data Sources", forget_password).view().params(r#"{"id": "a database connection's id or name"}"#), C::new("connection.test", "Test Connection", "Home › Add › Data Sources", test).view().params( r#"{"id"?: "a connection's id or name", "kind"?: "postgres"|"mysql"|"sqlserver"|"service"|"wms"|"wmts"|"wfs"|"wcs"|"ogcapi"|"arcgis"|"csw"|"stac"|"s3", "name"?: text, "group"?: text, "address"?: text, "options"?: "an unsaved connection, as connection.new (when no id)", "password"?: text, "accessKeyId"?: text, "secretAccessKey"?: text, "sessionToken"?: "as connection.login (logs in first)", "probe"?: "true: only the address the test reads, {fetch} (null for databases, which their driver tests)", "answer"?: "non-database kinds only: what that address returned, when the app fetched it (no network request then); databases ignore answer and test through their driver"}"#, ), @@ -38,27 +47,11 @@ pub fn commands() -> Vec { ] } -fn shown(c: &Connection) -> Value { +fn shown(s: &Session, c: &Connection) -> Value { let database = DATABASE_KINDS.contains(&c.kind.as_str()); let available = !database || !cfg!(target_arch = "wasm32"); - let logged_in = if c.kind == "s3" { - s3::connection(&c.name).is_some() - } else if database { - #[cfg(not(target_arch = "wasm32"))] - { - use rustgis_data::source::duckdb::{self, Database}; - match Database::parse(&c.address) { - Database::Postgres(d) | Database::Mysql(d) | Database::Sqlserver(d) => duckdb::has_password(&d), - Database::File(_) => false, - } - } - #[cfg(target_arch = "wasm32")] - { - false - } - } else { - false - }; + let logged_in = + if c.kind == "s3" { s3::connection(&c.name).is_some() } else { database && s.database_logins.has_session_password(&c.id) }; let add_with = "connection.browse {id}, then {id, node} for a child's node; run a dataset's add {command, params}"; let mut out = serde_json::to_value(c).unwrap_or(Value::Null); out["available"] = json!(available); @@ -66,6 +59,7 @@ fn shown(c: &Connection) -> Value { out["why"] = json!(DESKTOP_ONLY); } out["loggedIn"] = json!(logged_in); + out["remembered"] = json!(c.remembered); out["addWith"] = json!(add_with); out } @@ -91,7 +85,7 @@ pub(crate) fn waiting(s: &Session) -> Vec<(Connection, usize)> { for layer in s.project.maps.iter().flat_map(|m| &m.layers) { let Some(src) = layer.source.as_ref().filter(|src| src.login) else { continue }; let Some((kind, database)) = crate::connections::layer_database(src) else { continue }; - if let Some(c) = s.connections.for_database(kind, database) { + if let [c] = s.connections.matching_database(kind, database).as_slice() { *counts.entry(c.id.clone()).or_default() += 1; } } @@ -109,6 +103,22 @@ pub(crate) fn note_waiting(s: &mut Session) { for (c, n) in waiting(s) { s.notify(format!("{n} layer(s) read the database of the connection '{}' and need its password: right-click the connection in Data Sources › Connections, or one of the layers, and choose Log In…", c.name), false); } + let mut ambiguous = BTreeMap::, usize>::new(); + for l in s.project.maps.iter().flat_map(|m| &m.layers) { + let Some(src) = l.source.as_ref().filter(|src| src.login) else { continue }; + let Some((kind, database)) = crate::connections::layer_database(src) else { continue }; + let candidates = s.connections.matching_database(kind, database); + if candidates.len() > 1 { + let labels = candidates.iter().map(|c| format!("{} ({})", c.name, c.id)).collect(); + *ambiguous.entry(labels).or_default() += 1; + } + } + for (candidates, count) in ambiguous { + s.notify( + format!("{count} layer(s) need an explicit connection choice: {}. Use connection.login {{id}}.", candidates.join(", ")), + false, + ); + } } fn list(s: &mut Session, p: &Value) -> CmdResult { @@ -123,12 +133,16 @@ fn list(s: &mut Session, p: &Value) -> CmdResult { Ok(json!({"connections": s.connections.connections.iter() .filter(|c| kind.is_none_or(|k| c.kind == k) && group.is_none_or(|g| c.group.chars().flat_map(char::to_lowercase).eq(g.chars().flat_map(char::to_lowercase)))) .map(|c| { - let mut out = shown(c); + let mut out = shown(s, c); if let Some(&n) = waiting.get(&c.id) { out["waiting"] = json!(n); } out - }).collect::>() })) + }).collect::>(), + "loginRequests": s.database_logins.requests(s.opened).map(|r| json!({ + "reason":r.reason.name(), "database":crate::connections::database_label(r.kind, &r.key), + "connections":r.connections, "layers":r.layers.len() + })).collect::>() })) } fn new(s: &mut Session, p: &Value) -> CmdResult { @@ -142,11 +156,12 @@ fn new(s: &mut Session, p: &Value) -> CmdResult { }; let explicit = !c.name.trim().is_empty(); let (c, stripped) = s.connections.add(c, explicit)?; - Ok(shown_stripped(&c, stripped)) + resolve_project(s); + Ok(shown_stripped(s, &c, stripped)) } -fn shown_stripped(c: &Connection, stripped: bool) -> Value { - let mut out = shown(c); +fn shown_stripped(s: &Session, c: &Connection, stripped: bool) -> Value { + let mut out = shown(s, c); if stripped { out["stripped"] = json!(true); } @@ -169,14 +184,24 @@ fn edit(s: &mut Session, p: &Value) -> CmdResult { if let Some(v) = p.get("options") { c.options = options(v)?; } - let (c, stripped) = s.connections.update(&old.id, c)?; + let mut proposed = s.connections.clone(); + let (c, stripped) = proposed.update(&old.id, c)?; + if DATABASE_KINDS.contains(&old.kind.as_str()) && old.address != c.address { + delete_account(s, &old)?; + clear_identity(s, &old.id); + } + s.connections = proposed; + if old.address != c.address { + s.database_logins.clear_requests(); + resolve_project(s); + } if c.kind == "s3" && (old.name != c.name || old.address != c.address || old.options != c.options) { s3::disconnect(&old.name); if old.name != c.name { s3::disconnect(&c.name); } } - Ok(shown_stripped(&c, stripped)) + Ok(shown_stripped(s, &c, stripped)) } fn duplicate(s: &mut Session, p: &Value) -> CmdResult { @@ -187,47 +212,137 @@ fn duplicate(s: &mut Session, p: &Value) -> CmdResult { c.name = name.into(); } let (c, _) = s.connections.add(c, explicit)?; - Ok(shown(&c)) + s.database_logins.clear_requests(); + resolve_project(s); + Ok(shown(s, &c)) } fn remove(s: &mut Session, p: &Value) -> CmdResult { let c = found(s, p)?.clone(); - s.connections.remove(&c.id); + let mut proposed = s.connections.clone(); + proposed.remove(&c.id); + if DATABASE_KINDS.contains(&c.kind.as_str()) { + delete_account(s, &c)?; + clear_identity(s, &c.id); + } + s.connections = proposed; + s.database_logins.clear_requests(); + resolve_project(s); if c.kind == "s3" { s3::disconnect(&c.name); } Ok(json!({"removed": c.id})) } +fn delete_account(s: &mut Session, c: &Connection) -> Result<(), String> { + if s.database_logins.has_store() { + s.database_logins + .delete(&c.id) + .map_err(|_| "The saved password couldn't be deleted. The connection was left unchanged.".to_string()) + } else if c.remembered { + Err("The saved password couldn't be deleted. The connection was left unchanged.".into()) + } else { + Ok(()) + } +} + +fn clear_identity(s: &mut Session, id: &str) { + s.database_logins.forget_connection(id); + #[cfg(not(target_arch = "wasm32"))] + rustgis_data::source::duckdb::forget_connection_password(id); +} + +fn forget_password(s: &mut Session, p: &Value) -> CmdResult { + let c = found(s, p)?.clone(); + if !DATABASE_KINDS.contains(&c.kind.as_str()) { + return Err("only database connections have saved passwords".into()); + } + delete_account(s, &c)?; + s.connections.set_remembered(&c.id, false); + s.database_logins.forget_saved(&c.id); + #[cfg(not(target_arch = "wasm32"))] + { + rustgis_data::source::duckdb::forget_connection_password(&c.id); + if let Some(pw) = s.database_logins.session_password(&c.id) + && let Some((_, key)) = crate::connections::database_key(&c.address) + { + rustgis_data::source::duckdb::set_connection_password(&key, &c.id, Some(pw.expose())); + } + } + let mut c = c; + c.remembered = false; + Ok(shown(s, &c)) +} + #[derive(Default)] struct Reopened { opened: usize, failed: usize, + /// The server refused this attempt's password: desktop database reopen only. + #[cfg(not(target_arch = "wasm32"))] + refused: bool, + password_saved: Option, + /// A store save was asked for and failed: the fixed warning belongs to this case only. + save_failed: bool, } #[cfg(not(target_arch = "wasm32"))] -fn reopen(s: &mut Session, c: &Connection, password: &str) -> Reopened { +fn reopen(s: &mut Session, c: &Connection, saved: bool, all_unopened: bool) -> Reopened { let Some(key) = crate::connections::database_key(&c.address) else { return Reopened::default() }; let mut result = Reopened::default(); let mut first_error = None; for layer in s.project.maps.iter_mut().flat_map(|m| m.layers.iter_mut()) { - let Some(src) = layer.source.as_mut().filter(|src| src.login) else { continue }; + let Some(src) = layer.source.as_mut().filter(|src| src.login || all_unopened && !src.is_open()) else { continue }; let Some((kind, database)) = crate::connections::layer_database(src) else { continue }; if kind != key.0 || crate::connections::database_key(&crate::sources::redact(database)).as_ref() != Some(&key) { continue; } - if let Some(password_key) = crate::connections::database_key(database) { - rustgis_data::source::duckdb::remember_password(&password_key.1, password); + if saved && result.refused { + src.login = true; + src.error = first_error.clone(); + result.failed += 1; + continue; } src.error = None; match src.open() { Ok(()) => result.opened += 1, Err(e) => { result.failed += 1; + result.refused |= src.login; first_error.get_or_insert(e); } } } + if result.refused { + use crate::connections::{LoginReason, LoginRequest}; + if saved { + s.database_logins.reject(&c.id); + rustgis_data::source::duckdb::forget_connection_password(&c.id); + s.database_logins.drop_activation(&key.1); + } + let layers = s + .project + .maps + .iter() + .flat_map(|m| &m.layers) + .filter_map(|l| { + let src = l.source.as_ref().filter(|src| src.login)?; + let (_, db) = crate::connections::layer_database(src)?; + (crate::connections::database_key(&crate::sources::redact(db)).as_ref() == Some(&key)).then_some(l.id) + }) + .collect(); + s.database_logins.request(LoginRequest { + reason: LoginReason::Password, + kind: key.0, + key: key.1.clone(), + connections: vec![c.id.clone()], + layers, + opened: s.opened, + prompted: false, + }); + } else { + s.database_logins.clear_requests_for(key.0, &key.1); + } if result.opened > 0 { s.notify(format!("{} layer(s) from the connection '{}' opened again", result.opened, c.name), false); } @@ -246,6 +361,10 @@ fn reopen(s: &mut Session, c: &Connection, password: &str) -> Reopened { } fn login(s: &mut Session, c: &Connection, p: &Value) -> Result { + let remember = p.get("remember").map(|v| v.as_bool().ok_or("remember is a boolean")).transpose()?.unwrap_or(false); + if remember && !DATABASE_KINDS.contains(&c.kind.as_str()) { + return Err("only database connections can remember a password".into()); + } if DATABASE_KINDS.contains(&c.kind.as_str()) { #[cfg(target_arch = "wasm32")] { @@ -253,9 +372,39 @@ fn login(s: &mut Session, c: &Connection, p: &Value) -> Result } #[cfg(not(target_arch = "wasm32"))] { - let pw = ps(p, "password").filter(|t| !t.is_empty()).ok_or("password is required")?; - super::layer::database_param(&json!({"database": c.address, "password": pw})); - return Ok(reopen(s, c, pw)); + let key = crate::connections::database_key(&c.address).ok_or("not a database connection")?; + rustgis_data::source::duckdb::set_connection_password(&key.1, &c.id, None); + s.database_logins.drop_activation(&key.1); + s.database_logins.retry_saved(&c.id); + let typed = ps(p, "password").filter(|t| !t.is_empty()); + // A password typed for this address wins over any saved one (explicit input first). + let address_typed = typed.is_none() && rustgis_data::source::duckdb::has_explicit_password(&key.1); + let (password, saved) = match typed { + Some(pw) => { + s.database_logins.set_session_password(&c.id, pw); + (Some(rustgis_data::secrets::Secret::new(pw)), false) + } + None if address_typed => (None, false), + None => { + let (pw, saved) = credential(s, c)?.ok_or("password is required")?; + (Some(pw), saved) + } + }; + let all_unopened = s.database_logins.requests(s.opened).any(|r| r.kind == key.0 && r.key == key.1); + s.database_logins.select(key.0, &key.1, &c.id); + if let Some(password) = &password { + activate(s, c, password.expose()); + } + let mut result = reopen(s, c, saved, all_unopened); + if remember { + let stored = !result.refused && typed.is_some_and(|pw| s.database_logins.save(c, pw)); + if stored { + s.connections.set_remembered(&c.id, true); + } + result.save_failed = !stored && typed.is_some() && !result.refused; + result.password_saved = Some(stored); + } + return Ok(result); } } if c.kind != "s3" { @@ -292,7 +441,17 @@ fn login(s: &mut Session, c: &Connection, p: &Value) -> Result fn login_command(s: &mut Session, p: &Value) -> CmdResult { let c = found(s, p)?.clone(); let result = login(s, &c, p)?; - let mut out = shown(&c); + let mut c = c; + c.remembered = s.connections.find(&c.id).is_some_and(|c| c.remembered); + let mut out = shown(s, &c); + if let Some(saved) = result.password_saved { + out["passwordSaved"] = json!(saved); + // A refused password, or one nothing was typed for, isn't "available for this session": + // the fixed warning belongs to a store that couldn't keep a usable password. + if result.save_failed { + out["warning"] = json!("The password is available for this session, but wasn't remembered on this computer."); + } + } if result.opened + result.failed > 0 { out["reopened"] = json!(result.opened); out["failed"] = json!(result.failed); @@ -300,6 +459,33 @@ fn login_command(s: &mut Session, p: &Value) -> CmdResult { Ok(out) } +#[cfg(not(target_arch = "wasm32"))] +fn activate(s: &mut Session, c: &Connection, password: &str) { + if let Some((_, key)) = crate::connections::database_key(&c.address) { + rustgis_data::source::duckdb::set_connection_password(&key, &c.id, Some(password)); + s.database_logins.note_activation(&key, &c.id); + } +} + +#[cfg(not(target_arch = "wasm32"))] +fn credential(s: &mut Session, c: &Connection) -> Result, String> { + if let Some(pw) = s.database_logins.session_password(&c.id) { + return Ok(Some((pw.clone(), false))); + } + if !c.remembered || s.database_logins.is_rejected(&c.id) { + return Ok(None); + } + match s.database_logins.load(c) { + crate::connections::SavedPassword::Found(pw) => Ok(Some((pw, true))), + crate::connections::SavedPassword::Absent => { + s.connections.set_remembered(&c.id, false); + s.notify("The saved password is missing or doesn't match this connection.", true); + Ok(None) + } + crate::connections::SavedPassword::Unusable => Err("The saved password couldn't be used on this computer.".into()), + } +} + fn tested(s: &Session, p: &Value) -> Result { if ps(p, "id").is_some() { return Ok(found(s, p)?.clone()); @@ -367,9 +553,22 @@ fn answered(s: &mut Session, c: &Connection, text: &str) -> CmdResult { fn test(s: &mut Session, p: &Value) -> CmdResult { let c = tested(s, p)?; + if DATABASE_KINDS.contains(&c.kind.as_str()) && b(p, "probe").unwrap_or(false) { + return Ok(json!({"connection":c.id, "fetch":null})); + } let secrets = ["password", "accessKeyId", "secretAccessKey", "sessionToken"].iter().any(|k| ps(p, k).is_some_and(|v| !v.is_empty())); - if secrets { - login(s, &c, p)?; + if DATABASE_KINDS.contains(&c.kind.as_str()) { + if let Some(pw) = ps(p, "password").filter(|p| !p.is_empty()) + && !c.id.is_empty() + { + s.database_logins.set_session_password(&c.id, pw); + } + } else if secrets { + let mut input = p.clone(); + if let Some(o) = input.as_object_mut() { + o.remove("remember"); + } + login(s, &c, &input)?; } if c.kind == "s3" && !secrets && c.options.get("credentials").is_none_or(|v| v != "keys") { login(s, &c, &Value::Null)?; @@ -390,10 +589,18 @@ fn test(s: &mut Session, p: &Value) -> CmdResult { } let result = match c.kind.as_str() { "postgres" | "mysql" | "sqlserver" => { - if cfg!(target_arch = "wasm32") { - return Err(DESKTOP_ONLY.into()); + #[cfg(target_arch = "wasm32")] + return Err(DESKTOP_ONLY.into()); + #[cfg(not(target_arch = "wasm32"))] + { + if c.id.is_empty() { + super::layer::database_tables(s, &json!({"database":c.address,"password":ps(p,"password")}))? + } else { + let tables = connection_use::tables(s, &c)?; + let layers: Vec<_> = tables.iter().filter_map(|v| v["label"].as_str()).collect(); + json!({"count":layers.len(),"layers":layers}) + } } - super::layer::database_tables(s, &json!({"database": c.address}))? } "service" => super::service::layers(s, &json!({"url": c.address}))?, "wms" | "wmts" | "wfs" | "wcs" | "ogcapi" | "arcgis" => super::service::layers(s, &json!({"url": c.address, "type": c.kind}))?, diff --git a/crates/engine/src/cmd/items.rs b/crates/engine/src/cmd/items.rs index 4865117d6..a6e0b9c8e 100644 --- a/crates/engine/src/cmd/items.rs +++ b/crates/engine/src/cmd/items.rs @@ -177,10 +177,19 @@ pub fn commands() -> Vec { .params(r#"{"path": "*.rgistemplate", "name"?: template name}"#), C::new("project.fromTemplate", "Open Custom Template", "Home › Project", |s, p| { let text = read_text(p)?; - let mut proj = Project::from_import_json(&text).map_err(|e| e.replace("RustGIS project", "RustGIS template"))?; + super::connections::clear_active(); + let mut proj = match Project::from_import_json(&text) { + Ok(proj) => proj, + Err(error) => { + super::connections::restore_active(s); + return Err(error.replace("RustGIS project", "RustGIS template")); + } + }; proj.name = ps(p, "name").map(str::trim).filter(|n| !n.is_empty()).unwrap_or("Untitled").chars().take(200).collect(); super::project::reset_to(s, proj); s.path = None; + super::connections::resolve_project(s); + super::connections::note_waiting(s); Ok(Value::Null) }) .view() @@ -419,9 +428,16 @@ fn embedded_map(m: &Map, budget: &mut u64) -> Map { /// Adds maps from a map file, project or package. fn import_maps(s: &mut Session, p: &Value) -> CmdResult { let text = read_text(p)?; + super::connections::clear_active(); let maps: Vec = match serde_json::from_str::(&text) { Ok(m) => vec![m], - Err(_) => Project::from_import_json(&text).map_err(|_| "not a RustGIS map file, project or package".to_string())?.maps, + Err(_) => match Project::from_import_json(&text) { + Ok(project) => project.maps, + Err(_) => { + super::connections::restore_active(s); + return Err("not a RustGIS map file, project or package".into()); + } + }, }; let mut names = Vec::new(); for mut m in maps.into_iter().take(100) { @@ -436,6 +452,9 @@ fn import_maps(s: &mut Session, p: &Value) -> CmdResult { } // Repair what was read (rasters reopen, cameras clamp). s.project.normalize(); + super::connections::restore_active(s); + super::connections::resolve_project(s); + super::connections::note_waiting(s); if let Some(n) = names.last() && let Some(i) = s.project.maps.iter().position(|m| &m.name == n) { @@ -462,12 +481,11 @@ fn import_styles(s: &mut Session, p: &Value) -> CmdResult { } else if let Ok(m) = serde_json::from_str::(&text) { m.layers } else { - Project::from_import_json(&text) - .map_err(|_| "not a RustGIS layer file, map file or project".to_string())? - .maps - .into_iter() - .flat_map(|m| m.layers) - .collect() + // A project parse opens sources even though this import only takes their styles. + super::connections::clear_active(); + let project = Project::from_import_json(&text); + super::connections::restore_active(s); + project.map_err(|_| "not a RustGIS layer file, map file or project".to_string())?.maps.into_iter().flat_map(|m| m.layers).collect() }; let mut n = 0; for l in layers.into_iter().filter(|l| !l.is_raster() && l.geometry_type() != GeometryType::None).take(1000) { diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index 57adbf1c0..bd4b7840f 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -1372,15 +1372,6 @@ pub(super) fn database_tables(_s: &mut Session, p: &Value) -> CmdResult { } } -/// The geometry tables of a live database server, with the schema, name, column and type of -/// each (`connection.browse` groups them by schema). -#[cfg(not(target_arch = "wasm32"))] -pub(super) fn database_table_details(p: &Value) -> Result, String> { - let db = database_param(p).ok_or("database is required")?; - let live = LiveDb::connect(&db).ok_or("not a database server")?.map_err(|e| format!("{}: {e}", db.short()))?; - Ok(live.tables().map_err(|e| format!("{}: {e}", db.short()))?.iter().map(LiveTable::detail).collect()) -} - /// Adds the tables of a database (a PostgreSQL connection or a DuckDB file) that have a geometry /// column, or the one `layer` names. fn add_database(s: &mut Session, p: &Value) -> CmdResult { @@ -1500,16 +1491,6 @@ impl LiveTable { LiveTable::Sqlserver(t) => t.label(), } } - - /// The table as a browser lists it: its label for `layer.addDatabase`, and what it is made of. - fn detail(&self) -> Value { - let (schema, name, column, kind) = match self { - LiveTable::Postgis(t) => (&t.schema, &t.name, &t.column, &t.kind), - LiveTable::Mysql(t) => (&t.schema, &t.name, &t.column, &t.kind), - LiveTable::Sqlserver(t) => (&t.schema, &t.name, &t.column, &t.kind), - }; - json!({"label": self.label(), "schema": schema, "name": name, "column": column, "kind": kind}) - } } #[cfg(not(target_arch = "wasm32"))] @@ -1909,7 +1890,11 @@ pub(crate) fn add_layer_text(s: &mut Session, text: &str) -> CmdResult { layer.name = format!("{base} ({i})"); } let id = s.map_mut().add_layer(layer); + super::connections::clear_active(); s.project.normalize(); + super::connections::restore_active(s); + super::connections::resolve_project(s); + super::connections::note_waiting(s); s.active_layer = Some(id); Ok(json!({"layer": id})) } diff --git a/crates/engine/src/cmd/plugins.rs b/crates/engine/src/cmd/plugins.rs index bc00ee10b..87d7472a0 100644 --- a/crates/engine/src/cmd/plugins.rs +++ b/crates/engine/src/cmd/plugins.rs @@ -275,6 +275,7 @@ const NEVER: &[&str] = &[ "connection.duplicate", "connection.remove", "connection.login", + "connection.forgetPassword", "connection.test", "connection.browse", "s3.connect", diff --git a/crates/engine/src/cmd/project.rs b/crates/engine/src/cmd/project.rs index 7af12e005..84a0898a0 100644 --- a/crates/engine/src/cmd/project.rs +++ b/crates/engine/src/cmd/project.rs @@ -40,6 +40,9 @@ pub(crate) fn reset_to(s: &mut Session, p: Project) { } fn reset(s: &mut Session, p: Project) { + super::connections::clear_active(); + s.database_logins.clear_activations(); + s.database_logins.clear_requests(); // Kept until the command ends: a failure gives the project back with its history. s.put_aside(); s.project = p; @@ -74,19 +77,29 @@ const MAX_PACKAGE: u64 = 1 << 30; fn open(s: &mut Session, p: &Value) -> CmdResult { let path = PathBuf::from(super::ps(p, "path").ok_or("path is required")?); let is_package = path.extension().is_some_and(|e| e.eq_ignore_ascii_case("rgispkg")); - let proj = if is_package { - // Read at most a byte past the limit, so a huge file is refused rather than loaded. - use std::io::Read; - let mut bytes = Vec::new(); - std::fs::File::open(&path) - .and_then(|f| f.take(MAX_PACKAGE + 1).read_to_end(&mut bytes)) - .map_err(|e| format!("{}: {e}", path.display()))?; - if bytes.len() as u64 > MAX_PACKAGE { - return Err("the package is too large".into()); + super::connections::clear_active(); + let proj = (|| { + if is_package { + // Read at most a byte past the limit, so a huge file is refused rather than loaded. + use std::io::Read; + let mut bytes = Vec::new(); + std::fs::File::open(&path) + .and_then(|f| f.take(MAX_PACKAGE + 1).read_to_end(&mut bytes)) + .map_err(|e| format!("{}: {e}", path.display()))?; + if bytes.len() as u64 > MAX_PACKAGE { + return Err("the package is too large".into()); + } + Project::from_import_json(&super::items::package_text(&bytes)?) + } else { + Project::load(&path) + } + })(); + let proj = match proj { + Ok(proj) => proj, + Err(error) => { + super::connections::restore_active(s); + return Err(error); } - Project::from_import_json(&super::items::package_text(&bytes)?)? - } else { - Project::load(&path)? }; reset(s, proj); // A package opens as a new, unsaved project (saving writes a plain project file). @@ -95,13 +108,22 @@ fn open(s: &mut Session, p: &Value) -> CmdResult { } s.path = if is_package { None } else { Some(path) }; s.notify(format!("Opened {}", s.project.name), false); + super::connections::resolve_project(s); super::connections::note_waiting(s); Ok(Value::Null) } fn open_text(s: &mut Session, p: &Value) -> CmdResult { let text = super::ps(p, "text").ok_or("text is required")?; - let proj = if super::b(p, "package").unwrap_or(false) { Project::from_import_json(text)? } else { Project::from_json(text)? }; + super::connections::clear_active(); + let proj = if super::b(p, "package").unwrap_or(false) { Project::from_import_json(text) } else { Project::from_json(text) }; + let proj = match proj { + Ok(proj) => proj, + Err(error) => { + super::connections::restore_active(s); + return Err(error); + } + }; reset(s, proj); // The browser opens files by name and contents: the name still names the project. if let Some(file) = super::ps(p, "fileName") { @@ -109,6 +131,7 @@ fn open_text(s: &mut Session, p: &Value) -> CmdResult { } s.path = None; s.notify(format!("Opened {}", s.project.name), false); + super::connections::resolve_project(s); super::connections::note_waiting(s); Ok(Value::Null) } diff --git a/crates/engine/src/connections.rs b/crates/engine/src/connections.rs index 4791fdb6c..00a96ca81 100644 --- a/crates/engine/src/connections.rs +++ b/crates/engine/src/connections.rs @@ -2,7 +2,9 @@ //! secret: addresses are stripped of passwords, tokens and signatures before they are kept. use std::collections::{BTreeMap, BTreeSet}; +use std::sync::Arc; +use rustgis_data::secrets::{Secret, SecretError, SecretStore}; use serde::{Deserialize, Serialize}; use serde_json::{Value, json}; @@ -28,6 +30,11 @@ pub struct Connection { pub address: String, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub options: BTreeMap, + /// A database connection's password was saved in the system's secret store (never the + /// password itself, which is `DatabaseLogins`' to read). Set only by a login that asked to + /// remember it, and cleared when the saved password is forgotten or no longer fits. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub remembered: bool, } #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] @@ -84,6 +91,11 @@ pub fn layer_database(src: &rustgis_map::source::SourceLayer) -> Option<(&'stati } } +/// The password-free label for a driver's exact database key. +pub fn database_label(kind: &str, key: &str) -> String { + if kind == "postgres" && !key.contains("://") { format!("postgres:{key}") } else { key.to_string() } +} + fn same_name(a: &str, b: &str) -> bool { a.chars().flat_map(char::to_lowercase).eq(b.chars().flat_map(char::to_lowercase)) } @@ -121,6 +133,8 @@ pub fn clean(mut c: Connection) -> Result<(Connection, bool), String> { c.name = if c.kind == "s3" && c.address.is_empty() { "AWS S3".into() } else { sources::default_name(&c.address) }; } c.group = name_text(&c.group); + // Only a database connection has a password to remember. + c.remembered &= DATABASE_KINDS.contains(&c.kind.as_str()); if c.kind != "s3" && !c.options.is_empty() { return Err(format!("{} connections take no options", c.kind)); } @@ -157,13 +171,35 @@ pub fn clean(mut c: Connection) -> Result<(Connection, bool), String> { } impl Connections { - /// The first saved connection, in list order, of a database's kind whose address names it. + /// Every saved connection of a database's kind whose address names the database, in list order. + /// Only the wrapper, surrounding whitespace and password the address may carry are set aside: + /// the driver addresses are compared as text, so another form of the same server (a URI for a + /// connection string, reordered options, another name for the host) is another database. + pub fn matching_database(&self, kind: &str, database: &str) -> Vec<&Connection> { + match database_key(&sources::redact(database)) { + Some((k, key)) if k == kind => self.matching_key(kind, &key), + _ => Vec::new(), + } + } + + /// Every saved connection of a kind with this driver address (see [`database_key`]). + pub fn matching_key(&self, kind: &str, key: &str) -> Vec<&Connection> { + self.connections + .iter() + .filter(|c| c.kind == kind && database_key(&c.address).is_some_and(|(k, address)| k == kind && address == key)) + .collect() + } + + /// The saved connection a database comes from, when exactly one names it. Two that do leave + /// it to the user's choice (see [`Connections::matching_database`]): none is first. pub fn for_database(&self, kind: &str, database: &str) -> Option<&Connection> { - let key = database_key(&sources::redact(database)).filter(|k| k.0 == kind)?; - self.connections.iter().find(|c| c.kind == kind && database_key(&c.address).as_ref() == Some(&key)) + match self.matching_database(kind, database).as_slice() { + [only] => Some(*only), + _ => None, + } } - /// The saved connection a layer that reads a database comes from. + /// The saved connection a layer that reads a database comes from, when exactly one does. pub fn for_layer(&self, l: &rustgis_map::Layer) -> Option<&Connection> { let (kind, database) = layer_database(l.source.as_ref()?)?; self.for_database(kind, database) @@ -305,6 +341,8 @@ impl Connections { } self.next = next.checked_add(1).ok_or("connection ids exhausted")?; c.id = format!("c{next}"); + // A new connection, a copy or an import has no saved password: it is saved by a login. + c.remembered = false; self.connections.push(c.clone()); self.version = 1; self.generation = self.generation.wrapping_add(1); @@ -328,6 +366,9 @@ impl Connections { c.name = name; let slot = self.connections.iter_mut().find(|c| c.id == id).ok_or_else(|| format!("no connection '{id}'"))?; c.id = id.into(); + // The saved password belongs to the saved definition: an edit of the server or the user + // leaves it behind (the command deletes it first), and no edit claims one. + c.remembered = slot.remembered && slot.kind == c.kind && slot.address == c.address; *slot = c.clone(); self.generation = self.generation.wrapping_add(1); Ok((c, stripped)) @@ -339,6 +380,311 @@ impl Connections { self.generation = self.generation.wrapping_add(1); Some(c) } + + /// Says whether a database connection's password is saved; whether that changed the row. + pub fn set_remembered(&mut self, id: &str, on: bool) -> bool { + let Some(c) = self.connections.iter_mut().find(|c| c.id == id && DATABASE_KINDS.contains(&c.kind.as_str())) else { + return false; + }; + if c.remembered == on { + return false; + } + c.remembered = on; + self.generation = self.generation.wrapping_add(1); + true + } +} + +/// What a saved database password's lookup found. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum SavedPassword { + /// The password saved for this definition. + Found(Secret), + /// None: no account, or one saved for another server or user (or damaged). The connection's + /// marker should be cleared. + Absent, + /// The store couldn't be asked (none here, or locked, denied, unavailable): the marker says + /// what it said. + Unusable, +} + +/// Why a database's layers wait for the user. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum LoginReason { + /// More than one saved connection with a saved password names the database: the user picks + /// which one this session uses. + Choose, + /// No password is usable (the saved one was refused): the user types one. + Password, +} + +impl LoginReason { + pub fn name(self) -> &'static str { + match self { + LoginReason::Choose => "choose", + LoginReason::Password => "password", + } + } +} + +/// A database's layers waiting for a login the app should ask the user for. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct LoginRequest { + pub reason: LoginReason, + pub kind: &'static str, + /// The database as its driver names it, without a password. + pub key: String, + /// The saved connections it may be, by id. + pub connections: Vec, + /// The layers that wait, by id. + pub layers: Vec, + /// [`crate::Session::opened`] when it was made: a project opened since has other layers. + pub opened: u64, + /// The app has asked the user. + pub prompted: bool, +} + +/// The account a saved connection's password lives under in the system's secret store. +pub fn account(id: &str) -> String { + format!("connection/{id}") +} + +/// What the store keeps for a saved password: the definition it was saved for beside it, so an id +/// reused by another server (ids are sequential) never gets this one's password. +fn envelope(c: &Connection, password: &str) -> Secret { + Secret::new(json!({"kind": c.kind, "address": c.address, "password": password}).to_string()) +} + +/// The password in a stored envelope, if it was saved for exactly this definition. +fn password_in(c: &Connection, secret: &Secret) -> Option { + let v: Value = serde_json::from_str(secret.expose()).ok()?; + let o = v.as_object()?; + if o.len() != 3 || o.get("kind")?.as_str()? != c.kind || o.get("address")?.as_str()? != c.address { + return None; + } + o.get("password")?.as_str().filter(|p| !p.is_empty()).map(Secret::new) +} + +/// The database logins of this run: the system's secret store, the passwords given or loaded, which +/// connection was chosen for each database, and the logins the app should ask for. Never saved +/// with the project or the preferences, and never shown (its `Debug` counts, it doesn't print). +#[derive(Default)] +pub struct DatabaseLogins { + store: Option>, + /// Passwords given to `connection.login`, by connection id, for this session. + session: BTreeMap, + /// Saved passwords read from the store, by connection id. + saved: BTreeMap, + unavailable: BTreeSet, + /// The connection chosen for each database (kind and driver address) by a login. + selected: BTreeMap<(&'static str, String), String>, + /// Connections whose saved password the server refused this session. + rejected: BTreeSet, + /// The connection whose password the driver uses for each database address (driver key), + /// as set by a login or the project's resolution: what is set again when the driver's + /// passwords were cleared around reading a project's text. + activations: BTreeMap, + requests: Vec, +} + +impl std::fmt::Debug for DatabaseLogins { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("DatabaseLogins") + .field("store", &self.store.is_some()) + .field("session", &self.session.len()) + .field("saved", &self.saved.len()) + .field("selected", &self.selected.len()) + .field("rejected", &self.rejected.len()) + .field("requests", &self.requests.len()) + .finish() + } +} + +impl DatabaseLogins { + /// Gives the system's secret store to use (the desktop app's; `None` in the browser, on the + /// command line and in scripts, where a password lasts for the session). + pub fn set_store(&mut self, store: Option>) { + self.store = store; + self.saved.clear(); + self.unavailable.clear(); + } + + pub fn has_store(&self) -> bool { + self.store.is_some() + } + + /// Whether `id` has a password given this session. + pub fn has_session_password(&self, id: &str) -> bool { + self.session.contains_key(id) + } + + pub fn session_password(&self, id: &str) -> Option<&Secret> { + self.session.get(id) + } + + pub fn set_session_password(&mut self, id: &str, password: &str) { + self.session.insert(id.to_string(), Secret::new(password)); + } + + /// Reads the password saved for a definition (once: later calls use what was read). + pub fn load(&mut self, c: &Connection) -> SavedPassword { + if let Some(p) = self.saved.get(&c.id) { + return SavedPassword::Found(p.clone()); + } + if self.unavailable.contains(&c.id) { + return SavedPassword::Unusable; + } + let Some(store) = &self.store else { + self.unavailable.insert(c.id.clone()); + return SavedPassword::Unusable; + }; + match store.load(&account(&c.id)) { + Ok(Some(secret)) => match password_in(c, &secret) { + Some(p) => { + self.saved.insert(c.id.clone(), p.clone()); + SavedPassword::Found(p) + } + None => SavedPassword::Absent, + }, + Ok(None) => SavedPassword::Absent, + Err(_) => { + self.unavailable.insert(c.id.clone()); + SavedPassword::Unusable + } + } + } + + /// Saves a definition's password; whether it was saved. A saved password replaces an older + /// one and is trusted again if the server had refused the older one this session. + pub fn save(&mut self, c: &Connection, password: &str) -> bool { + let Some(store) = &self.store else { return false }; + if store.save(&account(&c.id), &envelope(c, password)).is_err() { + return false; + } + self.saved.insert(c.id.clone(), Secret::new(password)); + self.unavailable.remove(&c.id); + self.rejected.remove(&c.id); + true + } + + /// Deletes the account of a saved password (none to delete is success). Without a store the + /// error says so. + pub fn delete(&mut self, id: &str) -> Result<(), SecretError> { + let store = self.store.as_ref().ok_or_else(|| SecretError::Unavailable("no secret store".into()))?; + store.delete(&account(id))?; + self.saved.remove(id); + self.unavailable.remove(id); + self.rejected.remove(id); + Ok(()) + } + + /// Says the server refused the saved password of `id`: it is dropped from memory (the store + /// keeps it) and not tried again this session. + pub fn reject(&mut self, id: &str) { + self.saved.remove(id); + self.rejected.insert(id.to_string()); + } + + pub fn is_rejected(&self, id: &str) -> bool { + self.rejected.contains(id) + } + + /// The connection a login chose for a database, if any. + pub fn selection(&self, kind: &str, key: &str) -> Option<&str> { + self.selected.iter().find(|((k, d), _)| *k == kind && d == key).map(|(_, id)| id.as_str()) + } + + pub fn select(&mut self, kind: &'static str, key: &str, id: &str) { + self.selected.insert((kind, key.to_string()), id.to_string()); + } + + /// Forgets everything held for a connection that is going away or changing server: the + /// passwords given and read, the choice, the refusal, and the requests naming it. + pub fn forget_connection(&mut self, id: &str) { + self.session.remove(id); + self.saved.remove(id); + self.unavailable.remove(id); + self.rejected.remove(id); + self.selected.retain(|_, chosen| chosen != id); + self.activations.retain(|_, active| active != id); + self.requests.retain(|r| !r.connections.iter().any(|c| c == id)); + } + + /// Forgets the choice made for a database. + pub fn drop_selection(&mut self, kind: &str, key: &str) { + self.selected.retain(|(k, d), _| !(*k == kind && d == key)); + } + + /// The password a connection logs in with where it is known here: the one given this session, + /// else the saved one read (never reading the store). + pub fn known_password(&self, id: &str) -> Option<&Secret> { + self.session.get(id).or_else(|| self.saved.get(id)) + } + + /// Retries a lazy lookup on explicit login or Refresh, without retrying a refused password. + pub fn retry_saved(&mut self, id: &str) { + self.saved.remove(id); + self.unavailable.remove(id); + } + + /// Notes that `id`'s password is the one in use for a database address. + pub fn note_activation(&mut self, key: &str, id: &str) { + self.activations.insert(key.to_string(), id.to_string()); + } + + /// Notes that no connection's password is in use for a database address. + pub fn drop_activation(&mut self, key: &str) { + self.activations.remove(key); + } + + /// The database addresses with the connection whose password is in use for each. + pub fn activations(&self) -> Vec<(String, String)> { + self.activations.iter().map(|(k, id)| (k.clone(), id.clone())).collect() + } + + pub fn clear_activations(&mut self) { + self.activations.clear(); + } + + /// Forgets only the saved password of a connection, as the user asked to (a password given this + /// session stays). + pub fn forget_saved(&mut self, id: &str) { + self.saved.remove(id); + self.rejected.remove(id); + } + + /// Adds a request, replacing the one for the same database. The app is asked again only when + /// what it would ask has changed, within the same project (another project has other layers). + pub fn request(&mut self, mut request: LoginRequest) { + if let Some(old) = self.requests.iter().find(|r| (r.kind, &r.key) == (request.kind, &request.key)) { + request.prompted = + old.opened == request.opened && old.prompted && old.reason == request.reason && old.connections == request.connections; + } + self.requests.retain(|r| (r.kind, &r.key) != (request.kind, &request.key)); + self.requests.push(request); + } + + /// Drops the requests for a database (it logged in, or its layers went). + pub fn clear_requests_for(&mut self, kind: &str, key: &str) { + self.requests.retain(|r| (r.kind, r.key.as_str()) != (kind, key)); + } + + /// Drops every request: the project they came from is gone, or the saved connections changed. + pub fn clear_requests(&mut self) { + self.requests.clear(); + } + + /// The requests of the project opened as `opened`. + pub fn requests(&self, opened: u64) -> impl Iterator { + self.requests.iter().filter(move |r| r.opened == opened) + } + + /// The next request the app hasn't asked the user about, marked as asked. + pub fn next_unprompted(&mut self, opened: u64) -> Option { + let r = self.requests.iter_mut().find(|r| r.opened == opened && !r.prompted)?; + r.prompted = true; + Some(r.clone()) + } } #[cfg(test)] diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 352b490b1..c8acff3ac 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -465,6 +465,10 @@ pub struct Session { pub sources: sources::Sources, /// Saved connections (`connection.*`): kept with the app's preferences, not the project; never a secret. pub connections: connections::Connections, + /// The database logins of this run: the system's secret store (desktop app only), passwords + /// given and read, which saved connection each database uses, and the logins to ask for. Never + /// saved; see [`connections::DatabaseLogins`]. + pub database_logins: connections::DatabaseLogins, /// How rasters added from now on are drawn (`raster.setDefaultStyle`; kept with the app's /// preferences, not the project). pub raster_defaults: rustgis_map::RasterDefaults, @@ -671,6 +675,7 @@ impl Session { dirty: false, sources: sources::Sources::default(), connections: connections::Connections::default(), + database_logins: connections::DatabaseLogins::default(), raster_defaults: rustgis_map::RasterDefaults::default(), registry: Registry { commands: cmd::all() }, revision: 1, diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 25c905adf..59dd2123e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2,6 +2,14 @@ use serde_json::{Value, json}; use crate::{Project, Session, sample}; +#[cfg(not(target_arch = "wasm32"))] +#[path = "tests/remembered_database.rs"] +mod remembered_database; + +#[cfg(not(target_arch = "wasm32"))] +#[path = "tests/remembered_database_packets.rs"] +mod remembered_database_packets; + /// Keeps tests away from the user's installed plugins. fn isolate_plugins() { static ONCE: std::sync::Once = std::sync::Once::new(); @@ -1327,9 +1335,10 @@ fn hostile_params_never_panic() { json!({"transform": "affine", "crs": 32613, "pointsCrs": 4326, "cellSize": 1e-300, "path": "/nonexistent/x.tif", "points": [[0, 0, 1, 1], [1, 0, 2, 1], [0, 1, 1, 2]]}), json!({"points": [[0, 0, f64::MAX, f64::MAX], [1, 0, -f64::MAX, f64::MAX], [0, 1, f64::MAX, -f64::MAX]], "compare": true}), // Connections: junk kinds, options and secrets; no connection exists in a fresh session. - json!({"id": "c1", "kind": "s3", "address": "http://127.0.0.1:9", "name": "\u{7}", "group": 5, "options": {"credentials": "nope", "buckets": "NO_", "region": [1]}, "password": 7, "accessKeyId": "A", "secretAccessKey": 9}), + json!({"id": "c1", "kind": "s3", "address": "http://127.0.0.1:9", "name": "\u{7}", "group": 5, "options": {"credentials": "nope", "buckets": "NO_", "region": [1]}, "password": 7, "remember": [true], "accessKeyId": "A", "secretAccessKey": 9}), json!({"id": "", "kind": "postgres", "address": "postgres:host=127.0.0.1 port=9 password=x", "options": "x"}), json!({"kind": "wms", "address": "http://127.0.0.1:9/wms", "probe": "x", "answer": 7}), + json!({"id": {"unexpected":"object"}, "remember": "true", "password": []}), json!({"kind": "stac", "address": "http://127.0.0.1:9/stac", "answer": "\u{0}{"}), json!({"id": "c1", "node": "s3:/\u{7}", "probe": true, "answer": 7, "documents": "x"}), ]; @@ -9789,9 +9798,9 @@ fn database_layers_wait_for_their_connection_login() { assert_eq!(result["failed"], 2); assert!(t.map().layers.iter().take(2).all(|l| l.source.as_ref().unwrap().login)); assert_eq!(t.run("connection.list", &Value::Null).unwrap()["connections"][0]["waiting"], 2); - // Duplicate saved connections use list order for reporting, but either can log in. + // Duplicate addresses have no inferred connection identity; either can be selected explicitly. t.run("connection.new", &json!({"kind":"postgres","name":"Same database","address":dsn})).unwrap(); - assert_eq!(t.connections.for_layer(&t.map().layers[0]).unwrap().name, "Production GIS"); + assert!(t.connections.for_layer(&t.map().layers[0]).is_none()); let result = t.run("connection.login", &json!({"id":"Same database","password":"wrong"})).unwrap(); assert_eq!(result["failed"], 2); } @@ -9898,7 +9907,7 @@ fn connections_commands() { assert_eq!(list["connections"][0]["loggedIn"], true); assert!(!s.connections.to_json().contains("pw")); let copy = s.run("connection.duplicate", &json!({"id":db["id"]})).unwrap(); - assert_eq!(copy["loggedIn"], true, "the existing password store is shared by public DSN"); + assert_eq!(copy["loggedIn"], false, "a duplicate must not inherit another saved ID's session login"); let edited = s .run("connection.edit", &json!({"id":db["id"],"address":"postgres:host=127.0.0.1 port=9 dbname=connections_other user=u"})) .unwrap(); @@ -9906,7 +9915,7 @@ fn connections_commands() { s.run("connection.remove", &json!({"id":db["id"]})).unwrap(); let list = s.run("connection.list", &json!({"kind":"postgres"})).unwrap(); assert_eq!(list["connections"][0]["id"], copy["id"]); - assert_eq!(list["connections"][0]["loggedIn"], true, "removing a shortcut must not log out another connection using that DSN"); + assert_eq!(list["connections"][0]["loggedIn"], false); } s.run("connection.new", &json!({"kind":"s3","name":"conn-test-s3","options":{"credentials":"keys","buckets":"b11"}})).unwrap(); assert!(s.run("connection.test", &json!({"id":"conn-test-s3"})).unwrap_err().contains("log in first")); @@ -9930,6 +9939,7 @@ fn connections_commands() { "connection.duplicate", "connection.remove", "connection.login", + "connection.forgetPassword", "connection.test", "connection.browse", ] { diff --git a/crates/engine/src/tests/remembered_database.rs b/crates/engine/src/tests/remembered_database.rs new file mode 100644 index 000000000..22b18da12 --- /dev/null +++ b/crates/engine/src/tests/remembered_database.rs @@ -0,0 +1,113 @@ +use super::*; +use rustgis_data::secrets::{MemorySecrets, Secret, SecretError, SecretStore}; +use std::sync::Arc; + +fn session(store: Option>) -> Session { + let mut s = Session::new(Project::default()); + s.database_logins.set_store(store); + s +} +fn add(s: &mut Session, name: &str) -> String { + s.run( + "connection.new", + &json!({"kind":"postgres","name":name,"address":"postgres:host=127.0.0.1 port=1 user=remembered_lifecycle dbname=gis"}), + ) + .unwrap()["id"] + .as_str() + .unwrap() + .to_string() +} + +#[test] +fn remembered_database_opt_in_lifetime_and_identity() { + let store = Arc::new(MemorySecrets::new()); + let mut s = session(Some(store.clone())); + let id = add(&mut s, "First"); + s.run("connection.login", &json!({"id":id,"password":"sentinel-one"})).unwrap(); + assert_eq!(store.load(&format!("connection/{id}")).unwrap(), None); + let v = s.run("connection.login", &json!({"id":id,"password":"sentinel-one","remember":true})).unwrap(); + assert_eq!(v["passwordSaved"], true); + let envelope: Value = serde_json::from_str(store.load(&format!("connection/{id}")).unwrap().unwrap().expose()).unwrap(); + assert_eq!(envelope, json!({"kind":"postgres","address":s.connections.find(&id).unwrap().address,"password":"sentinel-one"})); + let other = add(&mut s, "Second"); + assert_eq!(s.run("connection.list", &Value::Null).unwrap()["connections"][1]["loggedIn"], false); + s.run("connection.login", &json!({"id":other,"password":"sentinel-two","remember":true})).unwrap(); + s.run("connection.edit", &json!({"id":id,"name":"Renamed","group":"Work"})).unwrap(); + assert!(s.connections.find(&id).unwrap().remembered); + let duplicate = s.run("connection.duplicate", &json!({"id":id})).unwrap(); + assert_eq!(duplicate["remembered"], false); + assert_eq!(duplicate["loggedIn"], false); + assert!(!s.connections.to_json().contains("sentinel")); + assert!(!s.project.saved_text().unwrap().0.contains("sentinel")); + s.run("connection.forgetPassword", &json!({"id":id})).unwrap(); + assert_eq!(store.load(&format!("connection/{id}")).unwrap(), None); + assert!(s.database_logins.has_session_password(&id)); + s.run("connection.forgetPassword", &json!({"id":id})).unwrap(); + s.run("connection.edit", &json!({"id":other,"address":"postgres:host=127.0.0.1 port=1 user=another dbname=gis"})).unwrap(); + assert!(!s.connections.find(&other).unwrap().remembered); + assert!(!s.database_logins.has_session_password(&other)); + assert_eq!(store.load(&format!("connection/{other}")).unwrap(), None); +} + +#[test] +fn remembered_database_store_failures_preserve_session() { + for store in [ + None, + Some(Arc::new(MemorySecrets::failing(SecretError::Denied)) as Arc), + Some(Arc::new(MemorySecrets::failing(SecretError::Unavailable("private backend payload".into()))) as Arc), + ] { + let mut s = session(store); + let id = add(&mut s, "Database"); + let v = s.run("connection.login", &json!({"id":id,"password":"session-sentinel","remember":true})).unwrap(); + assert_eq!(v["passwordSaved"], false); + assert_eq!(v["remembered"], false); + assert_eq!(v["loggedIn"], true); + assert_eq!(v["warning"], json!("The password is available for this session, but wasn't remembered on this computer.")); + assert!(!v.to_string().contains("private backend payload")); + s.run("connection.login", &json!({"id":id})).unwrap(); + } +} + +#[test] +fn remembered_database_invalid_envelope_and_reused_id() { + let store = Arc::new(MemorySecrets::new()); + let mut s = session(Some(store.clone())); + let id = add(&mut s, "Database"); + for raw in [ + "broken-secret-sentinel".to_string(), + json!({"kind":"postgres","address":"postgres:host=elsewhere","password":"stale-sentinel"}).to_string(), + json!({"kind":"postgres","address":s.connections.find(&id).unwrap().address,"password":"old-sentinel","extra":true}).to_string(), + ] { + store.save(&format!("connection/{id}"), &Secret::new(raw)).unwrap(); + s.connections.set_remembered(&id, true); + assert!(s.run("connection.login", &json!({"id":id})).is_err()); + assert!(!s.connections.find(&id).unwrap().remembered); + assert!(!s.notices.iter().any(|n| n.text.contains("sentinel"))); + } + for remember in [json!("true"), json!(1), Value::Null, json!({})] { + assert!(s.run("connection.login", &json!({"id":id,"password":"typed","remember":remember})).is_err()); + } +} + +#[test] +fn remembered_database_delete_failure_is_transactional() { + let store = Arc::new(MemorySecrets::new()); + let mut s = session(Some(store.clone())); + let id = add(&mut s, "Database"); + s.run("connection.login", &json!({"id":id,"password":"original","remember":true})).unwrap(); + let old = s.connections.to_json(); + s.database_logins.set_store(Some(Arc::new(MemorySecrets::failing(SecretError::Denied)))); + assert!(s.run("connection.edit", &json!({"id":id,"address":"postgres:host=changed user=other"})).is_err()); + assert_eq!(s.connections.to_json(), old); + assert!(s.run("connection.remove", &json!({"id":id})).is_err()); + assert_eq!(s.connections.to_json(), old); + assert!(s.run("connection.forgetPassword", &json!({"id":id})).is_err()); + assert_eq!(s.connections.to_json(), old); + s.database_logins.set_store(Some(store.clone())); + let second = add(&mut s, "Occupied"); + assert!(s.run("connection.edit", &json!({"id":id,"name":"Occupied","address":"postgres:host=changed"})).is_err()); + assert!(store.load(&format!("connection/{id}")).unwrap().is_some()); + s.run("connection.remove", &json!({"id":id})).unwrap(); + assert_eq!(store.load(&format!("connection/{id}")).unwrap(), None); + assert!(s.connections.find(&second).is_some()); +} diff --git a/crates/engine/src/tests/remembered_database_packets.rs b/crates/engine/src/tests/remembered_database_packets.rs new file mode 100644 index 000000000..6bd9c7f74 --- /dev/null +++ b/crates/engine/src/tests/remembered_database_packets.rs @@ -0,0 +1,266 @@ +use super::*; +use rustgis_data::secrets::{MemorySecrets, SecretStore}; +use std::io::{Read, Write}; +use std::sync::{ + Arc, + atomic::{AtomicBool, Ordering}, + mpsc, +}; + +struct Server { + address: String, + packets: mpsc::Receiver>, + stop: Arc, + worker: Option>, +} +impl Server { + fn new() -> Self { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + listener.set_nonblocking(true).unwrap(); + let stop = Arc::new(AtomicBool::new(false)); + let stopped = stop.clone(); + let (send, packets) = mpsc::channel(); + let worker = std::thread::spawn(move || { + while !stopped.load(Ordering::Relaxed) { + let (mut stream, _) = match listener.accept() { + Ok(pair) => pair, + Err(e) if e.kind() == std::io::ErrorKind::WouldBlock => { + std::thread::sleep(std::time::Duration::from_millis(2)); + continue; + } + Err(_) => break, + }; + // macOS accepts can inherit the listener's nonblocking flag. + stream.set_nonblocking(false).unwrap(); + stream.set_read_timeout(Some(std::time::Duration::from_secs(3))).unwrap(); + let mut size = [0; 4]; + if stream.read_exact(&mut size).is_err() { + continue; + } + let size = u32::from_be_bytes(size); + if !(4..=65536).contains(&size) { + continue; + } + if stream.read_exact(&mut vec![0; (size - 4) as usize]).is_err() { + continue; + } + if stream.write_all(b"R\0\0\0\x08\0\0\0\x03").is_err() { + continue; + } + let mut tag = [0]; + let mut size = [0; 4]; + if stream.read_exact(&mut tag).is_err() || tag[0] != b'p' || stream.read_exact(&mut size).is_err() { + continue; + } + let size = u32::from_be_bytes(size); + if !(4..=65536).contains(&size) { + continue; + } + let mut password = vec![0; (size - 4) as usize]; + if stream.read_exact(&mut password).is_err() { + continue; + } + let echoed = String::from_utf8_lossy(&password).trim_end_matches('\0').to_string(); + send.send(password).unwrap(); + let body = format!("SFATAL\0VFATAL\0C28P01\0Mrefused {echoed}\0\0"); + let _ = stream.write_all(b"E"); + let _ = stream.write_all(&((body.len() + 4) as u32).to_be_bytes()); + let _ = stream.write_all(body.as_bytes()); + } + }); + Self { + address: format!("postgres:host=127.0.0.1 port={port} user=packet_user dbname=gis sslmode=disable"), + packets, + stop, + worker: Some(worker), + } + } + #[track_caller] + fn take(&self) -> Vec { + self.packets.recv_timeout(std::time::Duration::from_secs(3)).unwrap() + } + fn empty(&self) { + assert!(self.packets.try_recv().is_err(), "unexpected password packet"); + } +} +impl Drop for Server { + fn drop(&mut self) { + self.stop.store(true, Ordering::Relaxed); + if let Some(worker) = self.worker.take() { + worker.join().unwrap(); + } + } +} +fn app(store: Arc, prefs: &str) -> Session { + let mut s = Session::new(Project::default()); + s.database_logins.set_store(Some(store)); + s.connections = crate::connections::Connections::from_json(prefs); + s +} +fn project(address: &str) -> String { + let mut p = world().project; + for layer in p.map_mut().layers.iter_mut().take(2) { + layer.source = Some(serde_json::from_value(json!({"postgis":{"database":address,"table":"roads","family":"Lines"}})).unwrap()); + } + serde_json::to_string(&p).unwrap() +} +fn remember(s: &mut Session, name: &str, address: &str, password: &str) -> String { + let id = + s.run("connection.new", &json!({"kind":"postgres","name":name,"address":address})).unwrap()["id"].as_str().unwrap().to_string(); + assert_eq!(s.run("connection.login", &json!({"id":id,"password":password,"remember":true})).unwrap()["passwordSaved"], true); + id +} + +// Driver passwords are process-global. Isolate packet scenarios from unrelated full-suite +// project resets (and ambient-password precedence tests) without mutating the parent's environment. +fn isolated(name: &str) -> bool { + isolated_with(name, &[]) +} + +/// The same isolation, with extra environment for the child (an ambient password, say). +fn isolated_with(name: &str, extra: &[(&str, &str)]) -> bool { + if std::env::var("RUSTGIS_PACKET_SCENARIO").as_deref() == Ok(name) { + return false; + } + let mut command = std::process::Command::new(std::env::current_exe().unwrap()); + command + .args(["--exact", &format!("tests::remembered_database_packets::{name}"), "--test-threads=1"]) + .env("RUSTGIS_PACKET_SCENARIO", name) + .env_remove("PGPASSWORD") + .env_remove("PGPASSFILE"); + for (key, value) in extra { + command.env(key, value); + } + let output = command.output().unwrap(); + assert!(output.status.success(), "isolated database scenario failed: {}", String::from_utf8_lossy(&output.stdout)); + true +} +#[test] +fn remembered_database_unique_packet_refusal_and_replacement() { + if isolated("remembered_database_unique_packet_refusal_and_replacement") { + return; + } + let server = Server::new(); + let store = Arc::new(MemorySecrets::new()); + let mut first = app(store.clone(), "{}"); + let id = remember(&mut first, "One", &server.address, "saved-packet-sentinel"); + let prefs = first.connections.to_json(); + let mut s = app(store.clone(), &prefs); + let text = project(&server.address); + s.run("project.openText", &json!({"text":text})).unwrap(); + assert!(s.map().layers.iter().take(2).all(|l| l.source.as_ref().is_some_and(|src| src.login)), "{:?}", s.notices); + assert_eq!(server.take(), b"saved-packet-sentinel\0"); + server.empty(); + assert!(s.map().layers.iter().take(2).all(|l| l.source.as_ref().unwrap().login)); + assert_eq!(s.run("connection.list", &Value::Null).unwrap()["loginRequests"][0]["reason"], "password"); + assert!(!s.notices.iter().any(|n| n.text.contains("saved-packet-sentinel"))); + assert!(s.map().layers.iter().take(2).all(|l| !l.source.as_ref().unwrap().error.as_ref().unwrap().contains("saved-packet-sentinel"))); + let saved = store.load(&format!("connection/{id}")).unwrap(); + s.run("project.openText", &json!({"text":text})).unwrap(); + server.empty(); + s.run("connection.login", &json!({"id":id,"password":"unchecked-new"})).unwrap(); + assert_eq!(server.take(), b"unchecked-new\0"); + assert_eq!(server.take(), b"unchecked-new\0"); + assert_eq!(store.load(&format!("connection/{id}")).unwrap(), saved); + assert!(s.database_logins.is_rejected(&id)); + // A refused password is not "available for this session", and the store keeps the old value. + let refused = s.run("connection.login", &json!({"id":id,"password":"refused-with-remember","remember":true})).unwrap(); + assert_eq!(refused["passwordSaved"], false, "{refused}"); + assert!(refused.get("warning").is_none(), "a refused password must not be called available: {refused}"); + assert_eq!(server.take(), b"refused-with-remember\0"); + assert_eq!(server.take(), b"refused-with-remember\0"); + assert_eq!(store.load(&format!("connection/{id}")).unwrap(), saved); + assert!(s.database_logins.is_rejected(&id)); + s.run("project.new", &Value::Null).unwrap(); + assert_eq!(s.run("connection.login", &json!({"id":id,"password":"checked-new","remember":true})).unwrap()["passwordSaved"], true); + assert!(!s.database_logins.is_rejected(&id)); +} +#[test] +fn remembered_database_ambiguity_requires_exact_id_packet() { + if isolated("remembered_database_ambiguity_requires_exact_id_packet") { + return; + } + let server = Server::new(); + let store = Arc::new(MemorySecrets::new()); + let mut first = app(store.clone(), "{}"); + let one = remember(&mut first, "One", &server.address, "first-sentinel"); + let two = remember(&mut first, "Two", &server.address, "second-sentinel"); + let mut s = app(store.clone(), &first.connections.to_json()); + let text = project(&server.address); + s.run("project.openText", &json!({"text":text})).unwrap(); + server.empty(); + let listing = s.run("connection.list", &Value::Null).unwrap(); + assert_eq!(listing["loginRequests"][0]["reason"], "choose"); + assert_eq!(listing["loginRequests"][0]["connections"], json!([one, two])); + assert!(s.connections.for_layer(&s.map().layers[0]).is_none()); + s.run("connection.login", &json!({"id":two})).unwrap(); + assert_eq!(server.take(), b"second-sentinel\0"); + server.empty(); + // Fresh sessions still see ambiguity; no prior automatic activation authorizes a choice. + let mut fresh = app(store.clone(), &first.connections.to_json()); + fresh.run("project.openText", &json!({"text":text})).unwrap(); + server.empty(); + assert_eq!(fresh.run("connection.list", &Value::Null).unwrap()["loginRequests"][0]["reason"], "choose"); + for id in [&one, &two] { + fresh.run("connection.forgetPassword", &json!({"id":id})).unwrap(); + } + let mut forgotten = app(store, &fresh.connections.to_json()); + forgotten.run("project.openText", &json!({"text":text})).unwrap(); + server.empty(); +} + +#[test] +fn remembered_database_exact_address_and_explicit_precedence_packets() { + if isolated("remembered_database_exact_address_and_explicit_precedence_packets") { + return; + } + let server = Server::new(); + let store = Arc::new(MemorySecrets::new()); + let mut seed = app(store.clone(), "{}"); + let id = remember(&mut seed, "Exact", &server.address, "exact-saved-sentinel"); + let prefs = seed.connections.to_json(); + let port = server.address.split_whitespace().find_map(|s| s.strip_prefix("port=")).unwrap(); + for address in [ + format!("postgresql://packet_user@127.0.0.1:{port}/gis?sslmode=disable"), + server.address.replace("user=packet_user dbname=gis", "dbname=gis user=packet_user"), + server.address.replace("user=packet_user", "user=other"), + ] { + let mut s = app(store.clone(), &prefs); + s.run("project.openText", &json!({"text":project(&address)})).unwrap(); + server.empty(); + } + let mut explicit = app(store, &prefs); + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + rustgis_data::source::duckdb::remember_password(&key, "explicit-sentinel"); + explicit.run("project.openText", &json!({"text":project(&server.address)})).unwrap(); + assert_eq!(server.take(), b"explicit-sentinel\0"); + assert_eq!(server.take(), b"explicit-sentinel\0"); + server.empty(); + // An explicit login without a password must not replace the address's own password with the + // saved one either: the address's wins, as it does when the project opens. + explicit.run("connection.login", &json!({"id":id})).unwrap(); + assert_eq!(server.take(), b"explicit-sentinel\0"); + assert_eq!(server.take(), b"explicit-sentinel\0"); + server.empty(); +} + +/// An explicitly initiated connection prefers the saved password to an ambient one (`PGPASSWORD`). +/// The ambient value is set in a child process: it is process-global, and the rest of the suite +/// must not see it. +#[test] +fn remembered_database_saved_password_wins_over_ambient_environment() { + if isolated_with("remembered_database_saved_password_wins_over_ambient_environment", &[("PGPASSWORD", "ambient-wrong-sentinel")]) { + return; + } + let server = Server::new(); + let store = Arc::new(MemorySecrets::new()); + let mut seed = app(store.clone(), "{}"); + let id = remember(&mut seed, "Ambient", &server.address, "ambient-saved-sentinel"); + let mut s = app(store, &seed.connections.to_json()); + // `connection.test` is explicitly initiated, so the driver may consult the ambient password: + // the saved one must be sent instead. + assert!(s.run("connection.test", &json!({"id":id})).is_err(), "the refusing server must fail the listing"); + assert_eq!(server.take(), b"ambient-saved-sentinel\0"); + server.empty(); +} diff --git a/crates/ui-egui/src/connectionui.rs b/crates/ui-egui/src/connectionui.rs index a1a2ec8f1..43e5d2fc5 100644 --- a/crates/ui-egui/src/connectionui.rs +++ b/crates/ui-egui/src/connectionui.rs @@ -1,7 +1,7 @@ -//! Named endpoint definitions and session-only logins, with web probes fetched off the frame. +//! Named endpoint definitions, optional remembered database passwords, and off-frame web probes. use egui::vec2; -use rustgis_engine::connections::{Connection, DATABASE_KINDS}; +use rustgis_engine::connections::{Connection, DATABASE_KINDS, LoginReason, LoginRequest, database_key, database_label}; use rustgis_engine::render::rustgis_tiles::{cache, s3}; use rustgis_engine::sources::Source; use serde_json::{Value, json}; @@ -20,6 +20,10 @@ pub struct State { pub credentials: String, pub profile: String, pub password: String, + pub remember: bool, + /// The current project's login prompt, never a password. + pub request: Option, + pub selected: Option, pub access_key: String, pub secret_key: String, pub session_token: String, @@ -108,7 +112,7 @@ pub(crate) fn keep_button(ui: &mut egui::Ui) -> bool { fn stripped_notice(app: &mut RustGisApp, v: &Value) { if v["stripped"] == true { - app.session.notify(tl!("A password or token in the address was left out: connections never keep secrets.").to_string(), false); + app.session.notify(tl!("A password or token in the address was left out of the saved connection definition.").to_string(), false); } } @@ -149,7 +153,13 @@ pub fn open_edit(app: &mut RustGisApp, id: &str) -> Result<(), String> { pub fn log_in(app: &mut RustGisApp, id: &str) { match app.run("ui.connection", json!({"id": id})) { - Ok(_) => app.dialogs.info = Some(tl!("Type the password or keys to log in for this session, then Save.").to_string()), + Ok(_) => { + app.dialogs.info = Some(if DATABASE_KINDS.contains(&app.ui.connection.kind.as_str()) { + tl!("Type the password, then Save. Remember on this computer is optional.").to_string() + } else { + tl!("Type the keys to log in for this session, then Save.").to_string() + }); + } Err(e) => app.session.notify(e, true), } } @@ -218,7 +228,126 @@ pub fn start_test(app: &mut RustGisApp) { app.dialogs.info = Some(tl!("Testing…").into()); } +/// A request remains authorized only for its original project and exact saved definitions. +fn candidates(app: &RustGisApp, request: &LoginRequest) -> Vec { + if request.opened != app.session.opened { + return Vec::new(); + } + request + .connections + .iter() + .filter_map(|id| { + let c = app.session.connections.find(id)?; + let (kind, key) = database_key(&c.address)?; + (kind == request.kind && key == request.key).then(|| c.clone()) + }) + .collect() +} + +fn password_request(app: &RustGisApp, id: &str) -> Option { + app.session + .database_logins + .requests(app.session.opened) + .find(|r| r.reason == LoginReason::Password && r.connections.iter().any(|c| c == id)) + .cloned() +} + +fn password_prompt(app: &mut RustGisApp, id: &str, request: Option, error: Option) { + match open_edit(app, id) { + Ok(()) => { + app.ui.connection.request = request; + app.dialogs.info = Some(tl!("The saved password could not be used. Type a password to try again; check Remember on this computer to replace the saved password.").into()); + app.dialogs.error = error; + } + Err(e) => app.session.notify(e, true), + } +} + +pub(crate) fn choose(app: &mut RustGisApp, request: LoginRequest) { + if candidates(app, &request).is_empty() { + return; + } + app.ui.connection = State { request: Some(request), ..Default::default() }; + app.dialogs = crate::dialogs::Dialogs { open: Some("chooseConnection"), ..Default::default() }; +} + +pub(crate) fn chooser(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { + let Some(request) = app.ui.connection.request.clone() else { return true }; + let list = candidates(app, &request); + if list.is_empty() { + return true; + } + ui.label(tl!("Choose which saved connection to use for this database. This choice lasts for this session.")); + ui.label(database_label(request.kind, &request.key)); + egui::ScrollArea::vertical().max_height(300.0).show(ui, |ui| { + for c in &list { + ui.group(|ui| { + ui.selectable_value(&mut app.ui.connection.selected, Some(c.id.clone()), &c.name); + if !c.group.is_empty() { + ui.label(egui::RichText::new(&c.group).weak()); + } + ui.label(egui::RichText::new(&c.address).small().weak()); + }); + } + }); + let selected = app.ui.connection.selected.clone().filter(|id| list.iter().any(|c| &c.id == id)); + ui.horizontal(|ui| { + if ui.add_enabled(selected.is_some(), egui::Button::new(tl!("Use Connection"))).clicked() + && let Some(id) = selected + { + let result = app.run("connection.login", json!({"id":id})); + crate::connectiontree::refresh(app, Some(&id)); + let refused = password_request(app, &id); + if result.is_err() || refused.is_some() { + password_prompt(app, &id, refused.or(Some(request)), result.err()); + return false; + } + return true; + } + ui.button(tl!("Cancel")).clicked() + }) + .inner +} + +fn poll_requests(app: &mut RustGisApp) { + if let Some(request) = &app.ui.connection.request + && matches!(app.dialogs.open, Some("chooseConnection" | "editConnection")) + && (request.opened != app.session.opened || candidates(app, request).is_empty()) + { + app.dialogs.open = None; + app.ui.connection = State::default(); + } + // A pending request never displaces an unrelated dialog, including a form being edited. + if app.dialogs.open.is_some() { + return; + } + while let Some(request) = app.session.database_logins.next_unprompted(app.session.opened) { + if app.ui.connection.request.as_ref().is_some_and(|shown| { + shown.opened == request.opened + && shown.kind == request.kind + && shown.key == request.key + && shown.reason == request.reason + && shown.connections == request.connections + }) { + // A layer-menu prompt may have shown this request before automatic polling reached + // it. In particular, cancelling it must not immediately open it again. + continue; + } + let list = candidates(app, &request); + if list.is_empty() { + continue; + } + if request.reason == LoginReason::Choose || list.len() > 1 { + choose(app, request); + } else { + password_prompt(app, &list[0].id, Some(request), None); + } + break; + } +} + pub fn poll(app: &mut RustGisApp, ctx: &egui::Context) { + poll_requests(app); if !matches!(app.dialogs.open, Some("newConnection" | "editConnection")) { app.ui.connection.test = None; return; @@ -298,15 +427,34 @@ fn save(app: &mut RustGisApp) -> Result<(), String> { stripped_notice(app, &v); let mut login = json!({"id": v["id"]}); secrets(&app.ui.connection, &mut login); - if login.as_object().is_some_and(|p| p.len() > 1) - && let Err(e) = app.run("connection.login", login) - { - app.ui.connection.id = v["id"].as_str().map(str::to_owned); - app.dialogs.open = Some("editConnection"); - return Err(e); + if login.as_object().is_some_and(|p| p.len() > 1) { + if DATABASE_KINDS.contains(&app.ui.connection.kind.as_str()) { + login["remember"] = json!(app.ui.connection.remember); + } + let id = v["id"].as_str().unwrap_or("").to_owned(); + let result = app.run("connection.login", login); + crate::connectiontree::refresh(app, Some(&id)); + let refused = password_request(app, &id); + if result.is_err() || refused.is_some() { + app.ui.connection.id = Some(id); + app.ui.connection.password.clear(); + app.ui.connection.remember = false; + app.ui.connection.request = refused; + app.dialogs.open = Some("editConnection"); + return Err(result.err().unwrap_or_else(|| tl!("The password was refused. Type a password to try again.").into())); + } + // The command says why a checked Remember didn't keep the password; nothing is claimed + // when it did. + if let Some(warning) = result.as_ref().ok().and_then(|v| v["warning"].as_str()) { + app.session.notify(warning.to_string(), true); + } + } + if let Some(id) = v["id"].as_str() { + crate::connectiontree::refresh(app, Some(id)); } let st = &mut app.ui.connection; st.password.clear(); + st.remember = false; st.access_key.clear(); st.secret_key.clear(); st.session_token.clear(); @@ -314,9 +462,14 @@ fn save(app: &mut RustGisApp) -> Result<(), String> { } pub fn dialog(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { - ui.label( - egui::RichText::new(tl!("A named endpoint you can reuse. Passwords and keys are kept until RustGIS closes, never saved.")).weak(), - ); + let help = if DATABASE_KINDS.contains(&app.ui.connection.kind.as_str()) { + tl!( + "A named endpoint you can reuse. Passwords last for this session unless you choose to remember them in this computer's password store." + ) + } else { + tl!("A named endpoint you can reuse. Keys and tokens are kept until RustGIS closes, never saved.") + }; + ui.label(egui::RichText::new(help).weak()); let st = &mut app.ui.connection; let database = DATABASE_KINDS.contains(&st.kind.as_str()); let mut changed = false; @@ -341,14 +494,12 @@ pub fn dialog(app: &mut RustGisApp, ui: &mut egui::Ui) -> bool { ui.end_row(); ui.label(tl!("Password")); changed |= ui - .add( - egui::TextEdit::singleline(&mut st.password) - .password(true) - .hint_text(tl!("This session only (optional)")) - .desired_width(360.0), - ) + .add(egui::TextEdit::singleline(&mut st.password).password(true).hint_text(tl!("Optional")).desired_width(360.0)) .changed(); ui.end_row(); + ui.label(""); + changed |= ui.checkbox(&mut st.remember, tl!("Remember on this computer")).changed(); + ui.end_row(); } else if st.kind == "s3" { for (label, field, hint) in [ (tl!("Endpoint"), &mut st.address, tl!("Blank for Amazon S3")), @@ -434,6 +585,25 @@ pub fn open_connection(app: &mut RustGisApp, c: &Connection) -> Result<(), Strin if cfg!(target_arch = "wasm32") { return Err(tl!("Database connections are used by the desktop app").into()); } + if app.session.database_logins.has_session_password(&c.id) || c.remembered { + let result = app.run("connection.login", json!({"id":c.id})); + crate::connectiontree::refresh(app, Some(&c.id)); + let refused = password_request(app, &c.id); + if result.is_err() || refused.is_some() { + password_prompt(app, &c.id, refused, result.err()); + return Ok(()); + } + } + #[cfg(not(target_arch = "wasm32"))] + if !app.session.database_logins.has_session_password(&c.id) + && !c.remembered + && let Some((_, key)) = database_key(&c.address) + { + // Picking an ID without credentials must not inherit another ID's activation. + // An unowned, explicitly typed address password remains highest priority. + rustgis_engine::data::source::duckdb::set_connection_password(&key, &c.id, None); + app.session.database_logins.drop_activation(&key); + } ("database", Value::Null) } "service" => ("service", json!({"type":"auto"})), diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 849a814fc..27ad6d67b 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -73,6 +73,7 @@ impl Dialogs { "addUrl" => "addUrl", "addService" => "addService", "addDatabase" => "addDatabase", + "chooseConnection" => "chooseConnection", "s3" => "s3", "zarr" => "zarr", "exportLayer" => "exportLayer", @@ -158,6 +159,7 @@ pub fn show(app: &mut RustGisApp, ctx: &egui::Context) { "addDatabase" => "Add From Database", "newConnection" => "New Connection", "editConnection" => "Edit Connection", + "chooseConnection" => "Choose Connection", "s3" => "S3 Buckets", "zarr" => "Add Zarr Variable", "exportLayer" => "Export Features", @@ -183,7 +185,7 @@ pub fn show(app: &mut RustGisApp, ctx: &egui::Context) { ui.set_width(match which { "coordinateSystem" | "zarr" => 660.0, "addTiles" | "addUrl" | "addService" | "s3" => 560.0, - "newConnection" | "editConnection" => 520.0, + "newConnection" | "editConnection" | "chooseConnection" => 520.0, _ => 440.0, }); ui.spacing_mut().item_spacing = vec2(8.0, 8.0); @@ -250,6 +252,7 @@ pub fn show(app: &mut RustGisApp, ctx: &egui::Context) { "addService" => close = add_service(app, ui), "addDatabase" => close = add_database(app, ui), "newConnection" | "editConnection" => close = crate::connectionui::dialog(app, ui), + "chooseConnection" => close = crate::connectionui::chooser(app, ui), "s3" => crate::s3ui::dialog(app, ui), "zarr" => close = crate::zarrui::dialog(app, ui), "exportLayer" => close = export_layer(app, ui), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1bff96e66..96a5d11c3 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -894,6 +894,9 @@ impl RustGisApp { pub fn with_services(mut self, services: Services) -> Self { self.services = services; sourcespane::load_connections(&mut self); + // Saved database passwords live in the system's store, which the engine reaches through + // the session so the assistant and scripts see the same logins as the interface. + self.session.database_logins.set_store(self.services.secrets.clone()); if let Some(lang) = self.services.load_pref.as_ref().and_then(|load| load("language")) { self.ui.language = lang; } diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 254f20482..b4fc2726f 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -1802,18 +1802,38 @@ fn layer_menu(app: &mut RustGisApp, ui: &mut Ui, l: &Layer, reorder: bool) { let _ = app.run("ui.refreshService", json!({"layer":l.id})); ui.close(); } - let login = l - .source - .as_ref() - .is_some_and(|s| s.login) - .then(|| app.session.connections.for_layer(l)) - .flatten() - .map(|c| (c.id.clone(), c.name.clone())); - if let Some((id, name)) = login - && ui.button(crate::i18n::fmt("Log In to {name}…", &[("name", &name)])).clicked() - { - crate::connectionui::log_in(app, &id); - ui.close(); + let login = l.source.as_ref().and_then(|s| { + let (kind, database) = rustgis_engine::connections::layer_database(s)?; + let connections: Vec<_> = app.session.connections.matching_database(kind, database).into_iter().cloned().collect(); + let (_, key) = rustgis_engine::connections::database_key(database)?; + if !s.login && !app.session.database_logins.requests(app.session.opened).any(|r| r.kind == kind && r.key == key) { + return None; + } + Some((kind, key, connections)) + }); + if let Some((kind, key, connections)) = login { + if connections.len() == 1 { + let c = &connections[0]; + if ui.button(crate::i18n::fmt("Log In to {name}…", &[("name", &c.name)])).clicked() { + crate::connectionui::log_in(app, &c.id); + ui.close(); + } + } else if connections.len() > 1 && ui.button(tl!("Choose Connection…")).clicked() { + let opened = app.session.opened; + crate::connectionui::choose( + app, + rustgis_engine::connections::LoginRequest { + reason: rustgis_engine::connections::LoginReason::Choose, + kind, + key, + connections: connections.iter().map(|c| c.id.clone()).collect(), + layers: vec![l.id], + opened, + prompted: true, + }, + ); + ui.close(); + } } let items: &[(&str, &str)] = if l.is_raster() { &[("Zoom To Layer", "layer.zoomTo"), ("Symbology", "ui.pane.symbology")] diff --git a/crates/ui-egui/src/sourcespane.rs b/crates/ui-egui/src/sourcespane.rs index bcf88484e..d9887f488 100644 --- a/crates/ui-egui/src/sourcespane.rs +++ b/crates/ui-egui/src/sourcespane.rs @@ -252,6 +252,9 @@ fn connections(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, list: &[Connection } }); if ui.small_button("⟳").on_hover_text(tl!("Refresh every connection's lists")).clicked() { + for c in list.iter().filter(|c| DATABASE_KINDS.contains(&c.kind.as_str())) { + app.session.database_logins.retry_saved(&c.id); + } crate::connectiontree::refresh(app, None); } }); @@ -312,6 +315,10 @@ fn connection_header(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, c: &Connecti } else { ui.link(&c.name).on_hover_text(if c.kind == "s3" && c.address.is_empty() { tl!("Amazon S3") } else { &c.address }) }; + // The status precedes the kind: the narrow pane clips trailing row text. + if DATABASE_KINDS.contains(&c.kind.as_str()) && c.remembered { + ui.label(RichText::new(tl!("Saved password")).small().weak()); + } ui.label(RichText::new(tl!(crate::connectionui::label(&c.kind))).small().weak()); response.context_menu(|ui| { let mut items = vec![ @@ -336,7 +343,17 @@ fn connection_header(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, c: &Connecti crate::connectionui::log_in(app, &c.id); ui.close(); } + if DATABASE_KINDS.contains(&c.kind.as_str()) && c.remembered && ui.button(tl!("Forget Saved Password")).clicked() { + match app.run("connection.forgetPassword", json!({"id":c.id})) { + Ok(_) => crate::connectiontree::refresh(app, Some(&c.id)), + Err(e) => app.session.notify(e, true), + } + ui.close(); + } if ui.button(tl!("Refresh")).clicked() { + if DATABASE_KINDS.contains(&c.kind.as_str()) { + app.session.database_logins.retry_saved(&c.id); + } crate::connectiontree::refresh(app, Some(&c.id)); ui.close(); } @@ -353,8 +370,9 @@ fn connection_header(app: &mut RustGisApp, ui: &mut Ui, t: &Tokens, c: &Connecti ui.close(); } if ui.button(tl!("Remove")).clicked() { - if let Err(e) = app.run("connection.remove", json!({"id":c.id})) { - app.session.notify(e, true); + match app.run("connection.remove", json!({"id":c.id})) { + Ok(_) => crate::connectiontree::refresh(app, Some(&c.id)), + Err(e) => app.session.notify(e, true), } ui.close(); } diff --git a/crates/ui-egui/tests/remembered_database/mod.rs b/crates/ui-egui/tests/remembered_database/mod.rs new file mode 100644 index 000000000..fb6e4a3ae --- /dev/null +++ b/crates/ui-egui/tests/remembered_database/mod.rs @@ -0,0 +1,236 @@ +use super::*; +use egui::accesskit::Role; +use rustgis_engine::data::secrets::{MemorySecrets, SecretError, SecretStore}; +use rustgis_ui_egui::Services; +use std::sync::Arc; + +fn with_store(store: Arc) -> egui_kittest::Harness<'static, RustGisApp> { + let app = RustGisApp::with_sample().with_services(Services { secrets: Some(store), ..Default::default() }); + egui_kittest::Harness::builder().with_size(egui::vec2(1400.0, 860.0)).with_max_steps(64).build_ui_state( + |ui, app: &mut RustGisApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + app, + ) +} +fn settle(h: &mut egui_kittest::Harness<'_, RustGisApp>) { + for _ in 0..4 { + h.step(); + } +} +const ADDRESS: &str = "postgres:host=127.0.0.1 port=1 dbname=ui_remembered user=smoke sslmode=disable"; + +#[test] +fn remembered_database_checkbox_test_save_forget_and_reset() { + let store = Arc::new(MemorySecrets::new()); + let mut h = with_store(store.clone()); + h.state_mut().run("ui.connection", json!({"kind":"postgres"})).unwrap(); + h.state_mut().ui.connection.name = "Remembered UI".into(); + h.state_mut().ui.connection.address = ADDRESS.into(); + h.state_mut().ui.connection.password = "ui-password-sentinel".into(); + settle(&mut h); + assert!(!h.state().ui.connection.remember); + h.get_by_label("Remember on this computer").click(); + settle(&mut h); + assert!(h.state().ui.connection.remember); + h.get_by_role_and_label(Role::Button, "Test").click(); + settle(&mut h); + assert!(h.state().session.connections.connections.is_empty()); + assert_eq!(store.load("connection/c1").unwrap(), None); + h.get_by_role_and_label(Role::Button, "Save").click(); + settle(&mut h); + assert_eq!(h.state().dialogs.open, None); + let id = h.state().session.connections.find("Remembered UI").unwrap().id.clone(); + assert!(h.state().session.connections.find(&id).unwrap().remembered); + assert!(store.load(&format!("connection/{id}")).unwrap().is_some()); + h.state_mut().run("ui.connection", json!({"id":id})).unwrap(); + settle(&mut h); + assert!(!h.state().ui.connection.remember); + assert!(h.state().ui.connection.password.is_empty()); + h.get_by_role_and_label(Role::Button, "Cancel").click(); + h.state_mut().run("ui.pane.sources", json!({})).unwrap(); + settle(&mut h); + h.get_by_label("Remembered UI").click_secondary(); + settle(&mut h); + h.get_by_label("Forget Saved Password").click(); + settle(&mut h); + assert_eq!(store.load(&format!("connection/{id}")).unwrap(), None); + assert!(!h.state().session.connections.find(&id).unwrap().remembered); + assert!(h.state().session.database_logins.has_session_password(&id)); +} + +#[test] +fn remembered_database_failed_store_has_visible_notice() { + let mut h = with_store(Arc::new(MemorySecrets::failing(SecretError::Denied))); + h.state_mut().run("ui.connection", json!({"kind":"postgres"})).unwrap(); + h.state_mut().ui.connection.name = "Session only UI".into(); + h.state_mut().ui.connection.address = ADDRESS.into(); + h.state_mut().ui.connection.password = "session-only-sentinel".into(); + settle(&mut h); + h.get_by_label("Remember on this computer").click(); + settle(&mut h); + h.get_by_role_and_label(Role::Button, "Save").click(); + settle(&mut h); + assert_eq!(h.state().dialogs.open, None); + let c = h.state().session.connections.find("Session only UI").unwrap(); + assert!(!c.remembered); + assert!(h.state().session.database_logins.has_session_password(&c.id)); + assert!(h.state().session.notices.iter().any(|n| n.error)); + assert!(!h.state().session.notices.iter().any(|n| n.text.contains("session-only-sentinel"))); +} + +#[test] +fn remembered_database_chooser_cancel_selection_and_project_scope() { + let store = Arc::new(MemorySecrets::new()); + let mut seed = with_store(store.clone()); + for name in ["First database", "Second database"] { + let id = seed.state_mut().run("connection.new", json!({"kind":"postgres","name":name,"address":ADDRESS})).unwrap()["id"] + .as_str() + .unwrap() + .to_string(); + seed.state_mut().run("connection.login", json!({"id":id,"password":"chooser-ui-sentinel","remember":true})).unwrap(); + } + let prefs = seed.state().session.connections.to_json(); + let mut project = seed.state().session.project.clone(); + project.map_mut().layers[0].source = + Some(serde_json::from_value(json!({"postgis":{"database":ADDRESS,"table":"roads","family":"Lines"}})).unwrap()); + let text = serde_json::to_string(&project).unwrap(); + let mut h = with_store(store); + h.state_mut().session.connections = rustgis_engine::connections::Connections::from_json(&prefs); + h.state_mut().run("project.openText", json!({"text":text})).unwrap(); + settle(&mut h); + assert_eq!(h.state().dialogs.open, Some("chooseConnection")); + assert!( + h.state().session.database_logins.selection("postgres", &rustgis_engine::connections::database_key(ADDRESS).unwrap().1).is_none() + ); + h.get_by_role_and_label(Role::Button, "Cancel").click(); + settle(&mut h); + assert_eq!(h.state().dialogs.open, None); + assert!(!h.state().session.map().layers[0].source.as_ref().unwrap().is_open()); + h.state_mut().run("project.openText", json!({"text":text})).unwrap(); + settle(&mut h); + h.query_all_by_label("Second database").next().unwrap().click(); + settle(&mut h); + h.get_by_role_and_label(Role::Button, "Use Connection").click(); + settle(&mut h); + let key = rustgis_engine::connections::database_key(ADDRESS).unwrap().1; + assert_eq!(h.state().session.database_logins.selection("postgres", &key), Some("c2")); + h.state_mut().run("project.new", json!({})).unwrap(); + settle(&mut h); + assert_ne!(h.state().dialogs.open, Some("chooseConnection")); + assert!(h.state().session.database_logins.requests(h.state().session.opened).next().is_none()); +} + +#[test] +fn remembered_database_refused_saved_login_opens_empty_unchecked_form() { + if std::env::var_os("RUSTGIS_UI_REFUSAL_CHILD").is_none() { + let output = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "remembered_database::remembered_database_refused_saved_login_opens_empty_unchecked_form", + "--test-threads=1", + ]) + .env("RUSTGIS_UI_REFUSAL_CHILD", "1") + .env_remove("PGPASSWORD") + .env_remove("PGPASSFILE") + .output() + .unwrap(); + assert!(output.status.success(), "isolated UI refusal failed: {}", String::from_utf8_lossy(&output.stdout)); + return; + } + use std::io::{Read, Write}; + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let worker = std::thread::spawn(move || { + for _ in 0..2 { + let (mut stream, _) = listener.accept().unwrap(); + stream.set_read_timeout(Some(std::time::Duration::from_secs(3))).unwrap(); + let mut size = [0; 4]; + if stream.read_exact(&mut size).is_err() { + continue; + } + let size = u32::from_be_bytes(size); + assert!((4..=65536).contains(&size)); + if stream.read_exact(&mut vec![0; (size - 4) as usize]).is_err() { + continue; + } + stream.write_all(b"R\0\0\0\x08\0\0\0\x03").unwrap(); + let mut tag = [0]; + if stream.read_exact(&mut tag).is_err() { + continue; + } + let mut size = [0; 4]; + if tag[0] != b'p' || stream.read_exact(&mut size).is_err() { + continue; + } + let size = u32::from_be_bytes(size); + assert!((4..=65536).contains(&size)); + let mut password = vec![0; (size - 4) as usize]; + stream.read_exact(&mut password).unwrap(); + assert_eq!(password, b"refused-ui-sentinel\0"); + let body = b"SFATAL\0VFATAL\0C28P01\0Mrefused refused-ui-sentinel\0\0"; + stream.write_all(b"E").unwrap(); + stream.write_all(&((body.len() + 4) as u32).to_be_bytes()).unwrap(); + stream.write_all(body).unwrap(); + } + }); + let store = Arc::new(MemorySecrets::new()); + let address = format!("postgres:host=127.0.0.1 port={port} dbname=gis user=refused_ui sslmode=disable"); + let mut seed = with_store(store.clone()); + seed.state_mut().run("connection.new", json!({"kind":"postgres","name":"Refused UI","address":address})).unwrap(); + seed.state_mut().run("connection.login", json!({"id":"c1","password":"refused-ui-sentinel","remember":true})).unwrap(); + let prefs = seed.state().session.connections.to_json(); + let mut project = seed.state().session.project.clone(); + project.map_mut().layers[0].source = + Some(serde_json::from_value(json!({"postgis":{"database":address,"table":"roads","family":"Lines"}})).unwrap()); + let mut h = with_store(store); + h.state_mut().session.connections = rustgis_engine::connections::Connections::from_json(&prefs); + h.state_mut().run("project.openText", json!({"text":serde_json::to_string(&project).unwrap()})).unwrap(); + worker.join().unwrap(); + settle(&mut h); + assert_eq!(h.state().dialogs.open, Some("editConnection")); + assert!(h.state().ui.connection.password.is_empty()); + assert!(!h.state().ui.connection.remember); + assert!(h.state().dialogs.info.is_some()); + assert!(h.state().session.database_logins.is_rejected("c1")); + assert!(!h.state().session.notices.iter().any(|n| n.text.contains("refused-ui-sentinel"))); +} + +#[test] +fn remembered_database_recording_history_recent_and_preferences_never_get_sentinel() { + use std::collections::HashMap; + use std::sync::Mutex; + let store = Arc::new(MemorySecrets::new()); + let prefs = Arc::new(Mutex::new(HashMap::::new())); + let (load, save) = (prefs.clone(), prefs.clone()); + let mut app = RustGisApp::with_sample().with_services(Services { + secrets: Some(store.clone()), + load_pref: Some(Box::new(move |k| load.lock().unwrap().get(k).cloned())), + save_pref: Some(Box::new(move |k, v| { + save.lock().unwrap().insert(k.into(), v.into()); + })), + ..Default::default() + }); + app.run("ui.taskRecord", json!({})).unwrap(); + app.run("connection.new", json!({"kind":"postgres","name":"No leak","address":ADDRESS})).unwrap(); + app.run("connection.login", json!({"id":"No leak","password":"boundary-sentinel","remember":true})).unwrap(); + let mut project = app.session.project.clone(); + let (text, _) = project.saved_text().unwrap(); + let outputs = [ + serde_json::to_string(&app.ws.recording).unwrap(), + app.run("sources.list", json!({})).unwrap().to_string(), + app.run("connection.list", json!({})).unwrap().to_string(), + app.run("gp.historyScript", json!({})).unwrap_or_default().to_string(), + serde_json::to_string(&*prefs.lock().unwrap()).unwrap(), + text, + format!("{:?}", app.session.database_logins), + format!("{:?}", app.session.notices), + ]; + for output in outputs { + assert!(!output.contains("boundary-sentinel"), "{output}"); + assert!(!output.contains("\"password\":\"boundary-sentinel\""), "{output}"); + } + assert!(store.load("connection/c1").unwrap().is_some()); +} diff --git a/crates/ui-egui/tests/smoke.rs b/crates/ui-egui/tests/smoke.rs index 645315738..8fb43e8cd 100644 --- a/crates/ui-egui/tests/smoke.rs +++ b/crates/ui-egui/tests/smoke.rs @@ -4,6 +4,9 @@ use egui_kittest::kittest::Queryable; use rustgis_ui_egui::RustGisApp; use serde_json::json; +#[cfg(not(target_arch = "wasm32"))] +mod remembered_database; + /// Builds a headless World Atlas UI for interaction tests. fn harness() -> egui_kittest::Harness<'static, RustGisApp> { egui_kittest::Harness::builder().with_size(egui::vec2(1400.0, 860.0)).with_max_steps(64).build_ui_state( diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index 62ecccf22..0dac677e4 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -163,17 +163,17 @@ type, and empty tables (such as the TIGER geocoder's) are left out. Features kee views: the table's integer primary key, else the row's place in the table (the rows of a view or a partitioned table without a key are numbered as read, so theirs can change between views). -The password is kept until RustGIS closes and never written to the project, which names the -connection without it. Without one, `PGPASSWORD` or libpq's password file (`~/.pgpass`, or -`%APPDATA%\postgresql\pgpass.conf`) serve when you add a table. A project keeps a streamed -table's connection and reads it again when it opens, sending only a password given in this -session: a project file could name any host, and must never get your own credentials. When the -server wants a password and the layer's database matches a saved connection, the project says -which connection is waiting. Right-click that connection in **Data Sources › Connections**, or a -waiting layer, and choose **Log In…**. Saving the password reopens every layer from that database. -Without a matching connection, connect again with **Add Data › Database…** (the layer reads again -when the project is next opened). A small table's features are kept in the project as they were -when added. +Passwords are never written to the project. By default, an entered password lasts until +RustGIS closes. For a named desktop connection, **Remember on this computer** is an unchecked +opt-in to the operating system's password store; see [Named connections](#named-connections) +for reconnect, selection and forgetting rules. Opening a project can use a remembered password +only for a unique, exact saved-address match, or an explicit session selection. Otherwise a +waiting layer offers **Log In…**, or a chooser when several remembered connections match. +Without a matching connection, connect again with **Add Data › Database…**. +`PGPASSWORD` or libpq's password file (`~/.pgpass`, or +`%APPDATA%\postgresql\pgpass.conf`) can serve when you explicitly add a table without a password; +opening a project never newly authorizes these ambient credentials. A small table's features +are kept in the project as they were when added. `layer.addSql` takes a connection as its `"database"` too, through DuckDB's `postgres` extension (installed the first time, like DuckDB Spatial), so a query names its tables plainly; the password @@ -231,10 +231,11 @@ the column's SRID (MySQL's per-column SRID, MariaDB's `REF_SYSTEM_ID`) or else i becomes a layer per type. Features keep their ids between views from the table's integer primary key; a table without one has its rows numbered as read, so theirs can change between views. -The password is kept until RustGIS closes and never written to the project; `MYSQL_PWD` serves -when you add a table without one, and a project opening sends only a password given in this -session, as for PostGIS. SQL queries (`layer.addSql`, the SQL pane) don't run on MySQL or MariaDB -databases. +Passwords are never written to the project and last for the session unless you opt in with +**Remember on this computer** for a named desktop connection. The same exact-address reconnect, +chooser and Forget rules in [Named connections](#named-connections) apply. `MYSQL_PWD` can serve when you explicitly +add a table without a password, but opening a project never newly authorizes it. +SQL queries (`layer.addSql`, the SQL pane) don't run on MySQL or MariaDB databases. ## SQL Server and Azure SQL @@ -278,13 +279,15 @@ read or sample locally instead of sending a viewport polygon to the server. A sm therefore miss features in a large sampled table. `geometry` layers and higher compatibility levels retain server-side viewport filtering. -The provider only reads data; edits are not written back to SQL Server. The password is kept -until RustGIS closes, never in the project. Secret-named query values such as `pwd` and -`access_token` are removed from saved connection addresses. `SQLCMDPASSWORD` serves only when adding a database, -not when reopening a project. A streamed layer reopening sends only credentials given this -session; Windows integrated sign-in also requires that same address to have been added this -session. Reconnect with **Add Data › Database…** when needed. SQL queries (`layer.addSql`, the SQL -pane) do not run on SQL Server connections. +The provider only reads data; edits are not written back to SQL Server. Passwords are never +written to the project and last for the session unless **Remember on this computer** is +checked for a named desktop connection. The [named-connection rules](#named-connections) govern +remembered reconnects and explicit selection. Secret-named query values such as `pwd` and +`access_token` are removed from saved connection addresses. `SQLCMDPASSWORD` serves only when +explicitly adding a database, not when opening a project. Windows integrated sign-in still +requires that same address to have been added this session: opening a project does not +authorize Windows identity. Reconnect with **Add Data › Database…** when needed. SQL queries +(`layer.addSql`, the SQL pane) do not run on SQL Server connections. ## Web Data @@ -457,29 +460,66 @@ names the command and parameters that add it (`service.add`, `layer.addDatabase` it left some out. Like `connection.test`, it reads the address itself on the desktop and the command line; with `probe: true` it returns the address to fetch, and the app passes what it fetched back as `answer` (and further `documents` when a service asks for them, as `next`). -Browsing never changes the project or the saved connections. - -Addresses are saved without passwords or tokens. `connection.login` remembers a database -`password`, or S3 `accessKeyId`, `secretAccessKey` and optional `sessionToken`, for this session -only, using the same credential stores as the existing add commands. With login parameters, -`connection.test` logs in first; a failed listing does not undo that session login. A database -login also reopens project layers that read the same password-free address and were refused for a -missing or wrong password. `connection.list` gives such waiting layers as `waiting`, and -`connection.login` gives `reopened` and `failed` when it tries any of them. A server that is down -or a missing table is not reported as waiting for a login. +Browsing never changes the project. When a saved password turns out to be missing, or to have been +saved for another address, the connection loses its **Saved password** marker; the stored entry +itself is left for **Forget Saved Password**, an address edit or a removal to delete. + +Addresses are saved without passwords or tokens. Database passwords entered through +`connection.login` are kept separately by connection ID for this session. +On desktop, **Remember on this computer** in the new, edit and login forms is always unchecked +initially. Checking it opts that submitted password into the operating system's password store; +**Test** never saves it. The command equivalent is +`connection.login {"id":"c1","password":"…","remember":true}`; `remember` defaults to false. +If the store is missing, unavailable or denies access, the login remains usable for the +session and a visible warning says it was not remembered (`passwordSaved:false` in the result). +The browser and command line have no OS-store hook and remain session-only. + +The stored envelope is bound to the connection's ID, kind and cleaned address, including its +username. Renaming or regrouping preserves it. Changing the address or username, or removing +the connection, deletes the old stored password before committing the change; if deletion +fails, the definition remains unchanged. Invalid edits do not delete a password. +Duplicating a connection copies neither its stored password nor its session login. +**Saved password** marks a remembered connection without reading the store just to display it. +**Forget Saved Password** (`connection.forgetPassword {"id":"c1"}`) deletes the stored value, +but leaves an explicitly entered session password and already-open layers alone. + +Projects remain address-only: they contain neither passwords nor saved connection IDs. +On opening a project, one exact matching remembered connection may reconnect automatically. +Matching strips passwords and outer wrappers/whitespace, but does not infer that URI and +libpq forms, reordered parameters, host aliases or different usernames mean the same server. +Several eligible remembered matches open a chooser with no default; cancel leaves the layers +unavailable. Choose a connection explicitly, or use `connection.login {"id":"c2"}` to select +its existing session or remembered password without exposing it. Selection lasts for this +session, not in the project. `connection.list` reports secret-free `loginRequests` with the +reason, database label, candidate IDs and affected-layer count; ambiguous layers are not +attributed to the first saved row. + +Explicit session input or selection takes precedence over a saved password; a saved password +takes precedence over existing ambient fallbacks on explicitly initiated connections. +Opening a project never newly authorizes environment/password-file credentials or Windows +identity. A positively refused saved password is not retried for the rest of the session, +and no other saved password is tried automatically. Its old OS-store value is retained. +The fresh login form has an empty password and Remember unchecked: an unchecked replacement +is session-only and does not overwrite or delete the old value; checking Remember and +successfully saving replaces it. A transport error or missing table is not a wrong-password +prompt. Use Forget to delete a previously saved password. + +A database login reopens waiting layers at the same password-free address; +`connection.list` reports them as `waiting`, and `connection.login` returns `reopened` and +`failed` counts. With login parameters, `connection.test` logs in first, but never persists a +password; a failed listing does not undo the session login. +S3 `accessKeyId`, `secretAccessKey` and optional `sessionToken` remain session-only. S3 profiles and environment credentials remain subject to the existing endpoint restrictions; editing an S3 name, endpoint or options disconnects its session login. S3 shares the name-based session registry used by the S3 dialog and `s3.connect`, so editing or removing a saved connection also affects a session entry of the same name created there. -Renaming clears the runtime entries under both the old and new names, so the renamed +Renaming clears runtime entries under both the old and new names, so the renamed connection cannot inherit an unrelated login at its destination name. Web-service logins are not supported. Plugins may list connections but cannot manage, log in -to, test or browse them. The in-app assistant asks for approval each time for those operations; -external assistants using MCP cannot run them. Even `probe: true` needs approval: connection -operations cannot be approved for the whole conversation. -Database passwords are shared by public DSN for the session, including existing layers and -other named connections using that DSN. Editing or removing a named connection does not revoke -that shared login; changing its address does not transfer the old password to the new address. +to, forget passwords for, test or browse them. The in-app assistant asks for fresh approval +each time for those operations, including Forget and scripted calls; external assistants +using MCP cannot run them. Even `probe: true` needs approval: connection operations cannot be +approved for the whole conversation. Desktop and web apps save connections with their preferences (`connections` in desktop `prefs.json`, `rustgis.connections` in web localStorage). The first load imports saved services, From 3cb95de69eb0f231ab2c5cf800176cf9591a4964 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Sat, 24 Oct 2026 01:44:16 +0200 Subject: [PATCH 2/8] Fix remembered database credential review issues --- crates/data/src/source/duckdb.rs | 23 +++ crates/engine/src/cmd/connection_browse.rs | 2 +- crates/engine/src/cmd/connection_use.rs | 52 +++-- crates/engine/src/cmd/connections.rs | 30 +-- crates/engine/src/connections.rs | 3 + .../engine/src/tests/remembered_database.rs | 17 ++ .../src/tests/remembered_database_packets.rs | 190 +++++++++++++++++- .../ui-egui/tests/remembered_database/mod.rs | 10 +- docs/user-guide/data.md | 13 +- 9 files changed, 301 insertions(+), 39 deletions(-) diff --git a/crates/data/src/source/duckdb.rs b/crates/data/src/source/duckdb.rs index ba8bf5f7d..24b1264d8 100644 --- a/crates/data/src/source/duckdb.rs +++ b/crates/data/src/source/duckdb.rs @@ -168,6 +168,29 @@ pub fn set_connection_password(dsn: &str, id: &str, password: Option<&str>) { } } +/// Whether this driver's exact address slot is currently owned by saved connection `id`. +/// Unlike runtime selection, this also accounts for a later explicitly typed address password. +pub fn has_connection_password(dsn: &str, id: &str) -> bool { + lock().get(dsn).is_some_and(|entry| entry.connection_id.as_deref() == Some(id)) +} + +/// Tries an explicitly supplied password, restoring the previous address slot and its ownership +/// only when the server refuses authentication. Other failures (including table listing) retain +/// the new password. Never holds the cache lock during network I/O. +pub fn try_connection_password(dsn: &str, id: &str, password: &str, run: impl FnOnce() -> Result) -> Result { + let previous = lock().insert(dsn.to_string(), Entry { password: Secret::new(password), connection_id: Some(id.to_string()) }); + let result = run(); + if matches!(&result, Err(DataError::Login(_))) { + let mut all = lock(); + if let Some(previous) = previous { + all.insert(dsn.to_string(), previous); + } else { + all.remove(dsn); + } + } + result +} + /// Takes away every password saved connection `id` activated. pub fn forget_connection_password(id: &str) { lock().retain(|_, e| e.connection_id.as_deref() != Some(id)); diff --git a/crates/engine/src/cmd/connection_browse.rs b/crates/engine/src/cmd/connection_browse.rs index 55f9dc581..5eff2b52c 100644 --- a/crates/engine/src/cmd/connection_browse.rs +++ b/crates/engine/src/cmd/connection_browse.rs @@ -230,7 +230,7 @@ pub(super) fn browse(s: &mut Session, p: &Value) -> CmdResult { fn database(s: &mut Session, c: &Connection) -> Result { use std::collections::BTreeMap; - let tables = super::connection_use::tables(s, c)?; + let tables = super::connection_use::tables(s, c, None)?; let text = |t: &Value, k: &str| t.get(k).and_then(Value::as_str).unwrap_or("").to_string(); let mut schemas: BTreeMap> = BTreeMap::new(); let truncated = tables.len() > MAX_CHILDREN; diff --git a/crates/engine/src/cmd/connection_use.rs b/crates/engine/src/cmd/connection_use.rs index af4eded1b..7772bcce9 100644 --- a/crates/engine/src/cmd/connection_use.rs +++ b/crates/engine/src/cmd/connection_use.rs @@ -7,12 +7,13 @@ use serde_json::json; use crate::Session; use crate::connections::Connection; -/// Geometry-table details shared by saved-ID Browse and Test. Call only after the probe return: -/// a probe must neither read the secret store nor connect to a database. -pub(super) fn tables(s: &mut Session, c: &Connection) -> Result, String> { +/// Geometry-table details shared by saved-ID Browse and Test. An explicitly typed Test password +/// replaces session state only if authentication wasn't refused; a later listing failure keeps it. +/// Call only after the probe return: a probe must neither read secrets nor connect. +pub(super) fn tables(s: &mut Session, c: &Connection, typed: Option<&str>) -> Result, String> { #[cfg(target_arch = "wasm32")] { - let _ = (s, c); + let _ = (s, c, typed); Err(crate::connections::DESKTOP_ONLY.into()) } #[cfg(not(target_arch = "wasm32"))] @@ -27,35 +28,52 @@ pub(super) fn tables(s: &mut Session, c: &Connection) -> Result, Stri Database::Sqlserver(key) => ("sqlserver", key), Database::File(_) => return Err("not a database server".into()), }; - // Clear even when loading fails: another saved ID's active password is not authority - // to read this connection. Passwords explicitly typed for the address remain intact. - duckdb::set_connection_password(key, &c.id, None); - s.database_logins.drop_activation(key); - let credential = if !s.database_logins.has_session_password(&c.id) && duckdb::has_explicit_password(key) { + let credential = if typed.is_some() { + // Keep session, selection and activation bookkeeping unchanged during the attempt. None } else { - super::credential(s, c)? + // Another saved ID's activation is not authority to read this connection. + // Passwords explicitly typed for the address remain intact. + duckdb::set_connection_password(key, &c.id, None); + s.database_logins.drop_activation(key); + if !s.database_logins.has_session_password(&c.id) && duckdb::has_explicit_password(key) { + None + } else { + super::credential(s, c)? + } }; - if credential.is_none() && s.database_logins.is_rejected(&c.id) && !duckdb::has_explicit_password(key) { + if typed.is_none() && credential.is_none() && s.database_logins.is_rejected(&c.id) && !duckdb::has_explicit_password(key) { super::request_password(s, c); return Err("password is required".into()); } let saved = credential.as_ref().is_some_and(|(_, saved)| *saved); - if let Some((password, _)) = &credential { + if typed.is_none() + && let Some((password, _)) = &credential + { super::activate(s, c, password.expose()); s.database_logins.select(kind, key, &c.id); } + let result = if let Some(pw) = typed { + duckdb::try_connection_password(key, &c.id, pw, || read_tables(&database)) + } else { + read_tables(&database) + }; + if let Some(pw) = typed + && !matches!(&result, Err(DataError::Login(_))) + { + s.database_logins.set_session_password(&c.id, pw); + s.database_logins.select(kind, key, &c.id); + s.database_logins.note_activation(key, &c.id); + } - match read_tables(&database) { + match result { Ok(tables) => { s.database_logins.clear_requests_for(kind, key); Ok(tables) } Err(error) => { - let error = match &credential { - Some((password, _)) => error.redact_secret(password.expose()), - None => error, - }; + let password = typed.or_else(|| credential.as_ref().map(|(pw, _)| pw.expose())).unwrap_or(""); + let error = error.redact_secret(password); if matches!(&error, DataError::Login(_)) { if saved { s.database_logins.reject(&c.id); diff --git a/crates/engine/src/cmd/connections.rs b/crates/engine/src/cmd/connections.rs index 30a721268..e2fd24fc8 100644 --- a/crates/engine/src/cmd/connections.rs +++ b/crates/engine/src/cmd/connections.rs @@ -39,7 +39,7 @@ pub fn commands() -> Vec { ), C::new("connection.forgetPassword", "Forget Saved Password", "Home › Add › Data Sources", forget_password).view().params(r#"{"id": "a database connection's id or name"}"#), C::new("connection.test", "Test Connection", "Home › Add › Data Sources", test).view().params( - r#"{"id"?: "a connection's id or name", "kind"?: "postgres"|"mysql"|"sqlserver"|"service"|"wms"|"wmts"|"wfs"|"wcs"|"ogcapi"|"arcgis"|"csw"|"stac"|"s3", "name"?: text, "group"?: text, "address"?: text, "options"?: "an unsaved connection, as connection.new (when no id)", "password"?: text, "accessKeyId"?: text, "secretAccessKey"?: text, "sessionToken"?: "as connection.login (logs in first)", "probe"?: "true: only the address the test reads, {fetch} (null for databases, which their driver tests)", "answer"?: "non-database kinds only: what that address returned, when the app fetched it (no network request then); databases ignore answer and test through their driver"}"#, + r#"{"id"?: "a connection's id or name", "kind"?: "postgres"|"mysql"|"sqlserver"|"service"|"wms"|"wmts"|"wfs"|"wcs"|"ogcapi"|"arcgis"|"csw"|"stac"|"s3", "name"?: text, "group"?: text, "address"?: text, "options"?: "an unsaved connection, as connection.new (when no id)", "password"?: "for a named database, authentication refusal restores prior session credentials; a later listing failure keeps the new password; never persisted", "accessKeyId"?: text, "secretAccessKey"?: text, "sessionToken"?: "as connection.login (logs in first)", "probe"?: "true: only the address the test reads, {fetch} (null for databases, which their driver tests)", "answer"?: "non-database kinds only: what that address returned, when the app fetched it (no network request then); databases ignore answer and test through their driver"}"#, ), C::new("connection.browse", "Browse Connection", "Home › Add › Data Sources", browse::browse).view().params( r#"{"id": "a connection's id or name", "node"?: "a child's node, as a listing gives it (the connection's top level when absent)", "probe"?: "true: only the address this level reads, {fetch} (null: read without a fetch, or by the database driver); an S3 connection logs in first, as connection.test does", "answer"?: "what that address returned, when the app fetched it", "documents"?: [{"url": "a further document a listing asked for (next)", "text": "its answer"}]} → {connection, node, children: [{key, name, what, detail?, node? (browse it), children? (listed already), add? {command, params}: the command that adds it}], truncated?, matched?, notes?, service? {url, type}, next? (fetch it, then ask again with answer and documents)}"#, @@ -51,7 +51,7 @@ fn shown(s: &Session, c: &Connection) -> Value { let database = DATABASE_KINDS.contains(&c.kind.as_str()); let available = !database || !cfg!(target_arch = "wasm32"); let logged_in = - if c.kind == "s3" { s3::connection(&c.name).is_some() } else { database && s.database_logins.has_session_password(&c.id) }; + if c.kind == "s3" { s3::connection(&c.name).is_some() } else { database && s.database_logins.known_password(&c.id).is_some() }; let add_with = "connection.browse {id}, then {id, node} for a child's node; run a dataset's add {command, params}"; let mut out = serde_json::to_value(c).unwrap_or(Value::Null); out["available"] = json!(available); @@ -262,11 +262,19 @@ fn forget_password(s: &mut Session, p: &Value) -> CmdResult { s.database_logins.forget_saved(&c.id); #[cfg(not(target_arch = "wasm32"))] { - rustgis_data::source::duckdb::forget_connection_password(&c.id); - if let Some(pw) = s.database_logins.session_password(&c.id) - && let Some((_, key)) = crate::connections::database_key(&c.address) + use rustgis_data::source::duckdb; + let key = crate::connections::database_key(&c.address).map(|(_, key)| key); + let was_active = key.as_ref().is_some_and(|key| duckdb::has_connection_password(key, &c.id)); + duckdb::forget_connection_password(&c.id); + if was_active + && let Some(key) = key + && !duckdb::has_explicit_password(&key) + && let Some(pw) = s.database_logins.session_password(&c.id) { - rustgis_data::source::duckdb::set_connection_password(&key, &c.id, Some(pw.expose())); + // Restore this ID's explicit session value only if it owned the active slot. + // An inactive ID must not replace another ID or a typed address password. + duckdb::set_connection_password(&key, &c.id, Some(pw.expose())); + s.database_logins.note_activation(&key, &c.id); } } let mut c = c; @@ -557,13 +565,7 @@ fn test(s: &mut Session, p: &Value) -> CmdResult { return Ok(json!({"connection":c.id, "fetch":null})); } let secrets = ["password", "accessKeyId", "secretAccessKey", "sessionToken"].iter().any(|k| ps(p, k).is_some_and(|v| !v.is_empty())); - if DATABASE_KINDS.contains(&c.kind.as_str()) { - if let Some(pw) = ps(p, "password").filter(|p| !p.is_empty()) - && !c.id.is_empty() - { - s.database_logins.set_session_password(&c.id, pw); - } - } else if secrets { + if !DATABASE_KINDS.contains(&c.kind.as_str()) && secrets { let mut input = p.clone(); if let Some(o) = input.as_object_mut() { o.remove("remember"); @@ -596,7 +598,7 @@ fn test(s: &mut Session, p: &Value) -> CmdResult { if c.id.is_empty() { super::layer::database_tables(s, &json!({"database":c.address,"password":ps(p,"password")}))? } else { - let tables = connection_use::tables(s, &c)?; + let tables = connection_use::tables(s, &c, ps(p, "password").filter(|pw| !pw.is_empty()))?; let layers: Vec<_> = tables.iter().filter_map(|v| v["label"].as_str()).collect(); json!({"count":layers.len(),"layers":layers}) } diff --git a/crates/engine/src/connections.rs b/crates/engine/src/connections.rs index 00a96ca81..24eceeb56 100644 --- a/crates/engine/src/connections.rs +++ b/crates/engine/src/connections.rs @@ -651,6 +651,9 @@ impl DatabaseLogins { pub fn forget_saved(&mut self, id: &str) { self.saved.remove(id); self.rejected.remove(id); + if !self.session.contains_key(id) { + self.activations.retain(|_, active| active != id); + } } /// Adds a request, replacing the one for the same database. The app is asked again only when diff --git a/crates/engine/src/tests/remembered_database.rs b/crates/engine/src/tests/remembered_database.rs index 22b18da12..ee3709ad3 100644 --- a/crates/engine/src/tests/remembered_database.rs +++ b/crates/engine/src/tests/remembered_database.rs @@ -111,3 +111,20 @@ fn remembered_database_delete_failure_is_transactional() { assert_eq!(store.load(&format!("connection/{id}")).unwrap(), None); assert!(s.connections.find(&second).is_some()); } + +#[test] +fn remembered_database_passwordless_login_reports_cached_credentials_per_id() { + let store = Arc::new(MemorySecrets::new()); + let mut seed = session(Some(store.clone())); + let id = add(&mut seed, "Remembered"); + let other = add(&mut seed, "Other"); + seed.run("connection.login", &json!({"id":id,"password":"status-sentinel","remember":true})).unwrap(); + let mut fresh = session(Some(store)); + fresh.connections = crate::connections::Connections::from_json(&seed.connections.to_json()); + assert_eq!(fresh.run("connection.list", &Value::Null).unwrap()["connections"][0]["loggedIn"], false); + assert_eq!(fresh.run("connection.login", &json!({"id":id})).unwrap()["loggedIn"], true); + let listing = fresh.run("connection.list", &Value::Null).unwrap(); + assert_eq!(listing["connections"][0]["loggedIn"], true); + assert_eq!(listing["connections"][1]["id"], other); + assert_eq!(listing["connections"][1]["loggedIn"], false); +} diff --git a/crates/engine/src/tests/remembered_database_packets.rs b/crates/engine/src/tests/remembered_database_packets.rs index 6bd9c7f74..db9a3158a 100644 --- a/crates/engine/src/tests/remembered_database_packets.rs +++ b/crates/engine/src/tests/remembered_database_packets.rs @@ -15,6 +15,11 @@ struct Server { } impl Server { fn new() -> Self { + Self::start(false) + } + + /// When authentication succeeds, fail the first catalogue query instead of the password. + fn start(authenticate: bool) -> Self { let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); let port = listener.local_addr().unwrap().port(); listener.set_nonblocking(true).unwrap(); @@ -63,10 +68,56 @@ impl Server { } let echoed = String::from_utf8_lossy(&password).trim_end_matches('\0').to_string(); send.send(password).unwrap(); - let body = format!("SFATAL\0VFATAL\0C28P01\0Mrefused {echoed}\0\0"); + if authenticate { + // AuthenticationOk and ReadyForQuery: the client's session setup is a + // simple Query containing three SET statements. + if stream.write_all(b"R\0\0\0\x08\0\0\0\0Z\0\0\0\x05I").is_err() { + continue; + } + let mut tag = [0]; + let mut size = [0; 4]; + if stream.read_exact(&mut tag).is_err() || tag[0] != b'Q' || stream.read_exact(&mut size).is_err() { + continue; + } + let size = u32::from_be_bytes(size); + if !(4..=65536).contains(&size) || stream.read_exact(&mut vec![0; (size - 4) as usize]).is_err() { + continue; + } + for _ in 0..3 { + stream.write_all(b"C\0\0\0\x08SET\0").unwrap(); + } + stream.write_all(b"Z\0\0\0\x05I").unwrap(); + // The catalogue lookup uses an extended query. Consume through Sync so + // the server's non-login error occurs after successful authentication. + loop { + let mut size = [0; 4]; + if stream.read_exact(&mut tag).is_err() || stream.read_exact(&mut size).is_err() { + break; + } + let size = u32::from_be_bytes(size); + if !(4..=65536).contains(&size) || stream.read_exact(&mut vec![0; (size - 4) as usize]).is_err() { + break; + } + if tag[0] == b'S' { + break; + } + } + } + let body = if authenticate { + format!("SERROR\0VERROR\0C42P01\0Mfixture listing failed {echoed}\0\0") + } else { + format!("SFATAL\0VFATAL\0C28P01\0Mrefused {echoed}\0\0") + }; let _ = stream.write_all(b"E"); let _ = stream.write_all(&((body.len() + 4) as u32).to_be_bytes()); let _ = stream.write_all(body.as_bytes()); + if authenticate { + let _ = stream.write_all(b"Z\0\0\0\x05I"); + // A catalogue error is nonfatal: keep the connection alive until the + // client closes it, rather than racing its error response with TCP EOF. + let mut terminate = [0; 5]; + let _ = stream.read_exact(&mut terminate); + } } }); Self { @@ -133,7 +184,15 @@ fn isolated_with(name: &str, extra: &[(&str, &str)]) -> bool { command.env(key, value); } let output = command.output().unwrap(); - assert!(output.status.success(), "isolated database scenario failed: {}", String::from_utf8_lossy(&output.stdout)); + let stdout = String::from_utf8_lossy(&output.stdout); + let mut summaries = stdout.lines().filter(|line| line.starts_with("test result:")); + let ran_one = summaries.next().is_some_and(|line| line.starts_with("test result: ok. 1 passed; 0 failed; 0 ignored;")) + && summaries.next().is_none(); + assert!( + output.status.success() && ran_one, + "isolated database scenario failed or did not run exactly one test:\n{stdout}\n{}", + String::from_utf8_lossy(&output.stderr) + ); true } #[test] @@ -264,3 +323,130 @@ fn remembered_database_saved_password_wins_over_ambient_environment() { assert_eq!(server.take(), b"ambient-saved-sentinel\0"); server.empty(); } + +#[test] +fn remembered_database_forget_preserves_active_owner_and_explicit_input() { + if isolated("remembered_database_forget_preserves_active_owner_and_explicit_input") { + return; + } + use rustgis_data::source::{duckdb, postgis}; + let server = Server::new(); + let store = Arc::new(MemorySecrets::new()); + let mut s = app(store.clone(), "{}"); + let a = remember(&mut s, "A", &server.address, "owner-a"); + let b = remember(&mut s, "B", &server.address, "owner-b"); + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + s.run("connection.forgetPassword", &json!({"id":a})).unwrap(); + assert_eq!(s.database_logins.selection("postgres", &key), Some(b.as_str())); + assert!(postgis::connect(&key, false).is_err()); + assert_eq!(server.take(), b"owner-b\0"); + assert_eq!(s.database_logins.session_password(&a).unwrap().expose(), "owner-a"); + assert!(store.load(&format!("connection/{a}")).unwrap().is_none()); + + duckdb::remember_password(&key, "address-typed"); + s.run("connection.forgetPassword", &json!({"id":a})).unwrap(); + assert!(duckdb::has_explicit_password(&key)); + assert!(postgis::connect(&key, false).is_err()); + assert_eq!(server.take(), b"address-typed\0"); + + s.run("connection.login", &json!({"id":b,"password":"owner-b"})).unwrap(); + // A loaded/active cache value can differ from the ID's latest explicit session value. + // Forget must restore the session value, not preserve the old owned slot by accident. + s.database_logins.set_session_password(&b, "session-b-new"); + s.run("connection.forgetPassword", &json!({"id":b})).unwrap(); + assert!(postgis::connect(&key, false).is_err()); + assert_eq!(server.take(), b"session-b-new\0"); + assert_eq!(s.database_logins.selection("postgres", &key), Some(b.as_str())); + server.empty(); +} + +#[test] +fn remembered_database_forget_saved_only_clears_its_activation() { + if isolated("remembered_database_forget_saved_only_clears_its_activation") { + return; + } + let server = Server::new(); + let store = Arc::new(MemorySecrets::new()); + let mut seed = app(store.clone(), "{}"); + let id = remember(&mut seed, "Saved", &server.address, "saved-only"); + let mut s = app(store, &seed.connections.to_json()); + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + s.run("connection.login", &json!({"id":id})).unwrap(); + assert!(!s.database_logins.has_session_password(&id)); + s.run("connection.forgetPassword", &json!({"id":id})).unwrap(); + assert!(!rustgis_data::source::duckdb::has_password(&key)); + assert!(!s.database_logins.activations().iter().any(|(_, active)| active == &id)); + server.empty(); +} + +#[test] +fn remembered_database_test_refusal_restores_prior_session_and_active_password() { + if isolated("remembered_database_test_refusal_restores_prior_session_and_active_password") { + return; + } + use rustgis_data::source::{duckdb, postgis}; + let server = Server::new(); + let store = Arc::new(MemorySecrets::new()); + let mut s = app(store.clone(), "{}"); + let a = remember(&mut s, "A", &server.address, "prior-a"); + let b = remember(&mut s, "B", &server.address, "prior-b"); + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + let saved = store.load(&format!("connection/{a}")).unwrap(); + for (active, expected) in [(Some(b.as_str()), "prior-b"), (Some(a.as_str()), "prior-a"), (None, "prior-address")] { + if let Some(id) = active { + s.run("connection.login", &json!({"id":id,"password":expected})).unwrap(); + } else { + duckdb::remember_password(&key, expected); + } + let selection = s.database_logins.selection("postgres", &key).map(str::to_string); + let activations = s.database_logins.activations(); + let error = s.run("connection.test", &json!({"id":a,"password":"test-refused","remember":true})).unwrap_err(); + assert!(!error.contains("test-refused")); + assert_eq!(server.take(), b"test-refused\0"); + assert_eq!(s.database_logins.session_password(&a).unwrap().expose(), "prior-a"); + assert_eq!(s.database_logins.selection("postgres", &key), selection.as_deref()); + assert_eq!(s.database_logins.activations(), activations); + assert_eq!(duckdb::has_explicit_password(&key), active.is_none()); + assert!(postgis::connect(&key, false).is_err()); + let mut packet = expected.as_bytes().to_vec(); + packet.push(0); + assert_eq!(server.take(), packet); + assert_eq!(store.load(&format!("connection/{a}")).unwrap(), saved); + assert!(!s.database_logins.is_rejected(&a)); + } + duckdb::set_connection_password(&key, &a, Some("cleanup-owned")); + duckdb::forget_connection_password(&a); + let mut fresh = app(Arc::new(MemorySecrets::new()), "{}"); + fresh.run("connection.new", &json!({"kind":"postgres","name":"Empty","address":server.address})).unwrap(); + assert!(fresh.run("connection.test", &json!({"id":"c1","password":"test-refused"})).is_err()); + assert_eq!(server.take(), b"test-refused\0"); + assert!(!fresh.database_logins.has_session_password("c1")); + assert!(fresh.database_logins.selection("postgres", &key).is_none()); + assert!(!duckdb::has_password(&key)); + server.empty(); +} + +#[test] +fn remembered_database_test_listing_failure_keeps_authenticated_session() { + if isolated("remembered_database_test_listing_failure_keeps_authenticated_session") { + return; + } + let server = Server::start(true); + let store = Arc::new(MemorySecrets::new()); + let mut s = app(store.clone(), "{}"); + let a = remember(&mut s, "A", &server.address, "prior-a"); + let b = remember(&mut s, "B", &server.address, "prior-b"); + let error = s.run("connection.test", &json!({"id":a,"password":"accepted-new","remember":true})).unwrap_err(); + assert!(error.contains("fixture listing failed"), "{error}"); + assert!(!error.contains("accepted-new")); + assert_eq!(server.take(), b"accepted-new\0"); + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + assert_eq!(s.database_logins.session_password(&a).unwrap().expose(), "accepted-new"); + assert_eq!(s.database_logins.selection("postgres", &key), Some(a.as_str())); + assert!(s.database_logins.activations().contains(&(key.clone(), a.clone()))); + assert!(s.database_logins.requests(s.opened).next().is_none()); + let envelope: Value = serde_json::from_str(store.load(&format!("connection/{a}")).unwrap().unwrap().expose()).unwrap(); + assert_eq!(envelope["password"], "prior-a"); + assert_eq!(s.database_logins.session_password(&b).unwrap().expose(), "prior-b"); + server.empty(); +} diff --git a/crates/ui-egui/tests/remembered_database/mod.rs b/crates/ui-egui/tests/remembered_database/mod.rs index fb6e4a3ae..41c95ba41 100644 --- a/crates/ui-egui/tests/remembered_database/mod.rs +++ b/crates/ui-egui/tests/remembered_database/mod.rs @@ -137,7 +137,15 @@ fn remembered_database_refused_saved_login_opens_empty_unchecked_form() { .env_remove("PGPASSFILE") .output() .unwrap(); - assert!(output.status.success(), "isolated UI refusal failed: {}", String::from_utf8_lossy(&output.stdout)); + let stdout = String::from_utf8_lossy(&output.stdout); + let mut summaries = stdout.lines().filter(|line| line.starts_with("test result:")); + let ran_one = summaries.next().is_some_and(|line| line.starts_with("test result: ok. 1 passed; 0 failed; 0 ignored;")) + && summaries.next().is_none(); + assert!( + output.status.success() && ran_one, + "isolated UI refusal failed or did not run exactly one test:\n{stdout}\n{}", + String::from_utf8_lossy(&output.stderr) + ); return; } use std::io::{Read, Write}; diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index 0dac677e4..21cff0061 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -481,7 +481,8 @@ fails, the definition remains unchanged. Invalid edits do not delete a password. Duplicating a connection copies neither its stored password nor its session login. **Saved password** marks a remembered connection without reading the store just to display it. **Forget Saved Password** (`connection.forgetPassword {"id":"c1"}`) deletes the stored value, -but leaves an explicitly entered session password and already-open layers alone. +but leaves an explicitly entered session password and already-open layers alone. Forgetting an +inactive connection never selects it or replaces another connection's active password. Projects remain address-only: they contain neither passwords nor saved connection IDs. On opening a project, one exact matching remembered connection may reconnect automatically. @@ -492,7 +493,8 @@ unavailable. Choose a connection explicitly, or use `connection.login {"id":"c2" its existing session or remembered password without exposing it. Selection lasts for this session, not in the project. `connection.list` reports secret-free `loginRequests` with the reason, database label, candidate IDs and affected-layer count; ambiguous layers are not -attributed to the first saved row. +attributed to the first saved row. `loggedIn` means this ID has a session password or an already +loaded remembered password available; it does not certify successful server authentication. Explicit session input or selection takes precedence over a saved password; a saved password takes precedence over existing ambient fallbacks on explicitly initiated connections. @@ -506,8 +508,11 @@ prompt. Use Forget to delete a previously saved password. A database login reopens waiting layers at the same password-free address; `connection.list` reports them as `waiting`, and `connection.login` returns `reopened` and -`failed` counts. With login parameters, `connection.test` logs in first, but never persists a -password; a failed listing does not undo the session login. +`failed` counts. `connection.test` never persists a password. For a named connection, if the +server positively refuses a password typed for that Test, the previous per-ID session credential, +selection and active address password are restored (or the attempted credential is removed if +none existed). +A failure after authentication, such as a table-listing error, keeps the new session login. S3 `accessKeyId`, `secretAccessKey` and optional `sessionToken` remain session-only. S3 profiles and environment credentials remain subject to the existing endpoint restrictions; editing an S3 name, endpoint or options disconnects its session login. S3 shares the name-based From 5b0d7d9cfc41f612aa4a2c7ef7228b41d108088a Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Sat, 24 Oct 2026 03:11:31 +0200 Subject: [PATCH 3/8] Resolve only the databases an incremental change names --- crates/engine/src/cmd/connection_logins.rs | 20 ++++++++++ crates/engine/src/cmd/connections.rs | 30 ++++++++++---- crates/engine/src/cmd/layer.rs | 4 +- .../src/tests/remembered_database_packets.rs | 40 ++++++++++++++++++- docs/user-guide/data.md | 4 +- 5 files changed, 87 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/cmd/connection_logins.rs b/crates/engine/src/cmd/connection_logins.rs index 97c84287c..e867e5849 100644 --- a/crates/engine/src/cmd/connection_logins.rs +++ b/crates/engine/src/cmd/connection_logins.rs @@ -47,11 +47,31 @@ pub(super) fn request_password(s: &mut Session, c: &Connection) { } } +/// Resolves every unopened database of the project: a project was opened or imported. pub(crate) fn resolve_project(s: &mut Session) { #[cfg(target_arch = "wasm32")] let _ = s; #[cfg(not(target_arch = "wasm32"))] + resolve(s, None); +} + +/// Resolves only the databases `keys` names: a saved definition or a single layer change can only +/// affect those, so an unrelated database that is down is not connected to again on every edit. +pub(crate) fn resolve_keys(s: &mut Session, keys: &[(&'static str, String)]) { + #[cfg(target_arch = "wasm32")] + let _ = (s, keys); + #[cfg(not(target_arch = "wasm32"))] + resolve(s, Some(keys)); +} + +#[cfg(not(target_arch = "wasm32"))] +fn resolve(s: &mut Session, only: Option<&[(&'static str, String)]>) { for ((kind, key), layers) in groups(s) { + if let Some(only) = only + && !only.iter().any(|(k, database)| *k == kind && *database == key) + { + continue; + } use rustgis_data::source::duckdb; if duckdb::has_explicit_password(&key) { s.database_logins.clear_requests_for(kind, &key); diff --git a/crates/engine/src/cmd/connections.rs b/crates/engine/src/cmd/connections.rs index e2fd24fc8..5620c152a 100644 --- a/crates/engine/src/cmd/connections.rs +++ b/crates/engine/src/cmd/connections.rs @@ -17,7 +17,7 @@ mod browse; mod logins; #[cfg(not(target_arch = "wasm32"))] use logins::request_password; -pub(crate) use logins::{clear_active, resolve_project, restore_active}; +pub(crate) use logins::{clear_active, resolve_keys, resolve_project, restore_active}; #[path = "connection_use.rs"] mod connection_use; @@ -156,7 +156,7 @@ fn new(s: &mut Session, p: &Value) -> CmdResult { }; let explicit = !c.name.trim().is_empty(); let (c, stripped) = s.connections.add(c, explicit)?; - resolve_project(s); + resolve_keys(s, &address_keys(&c.address)); Ok(shown_stripped(s, &c, stripped)) } @@ -192,8 +192,11 @@ fn edit(s: &mut Session, p: &Value) -> CmdResult { } s.connections = proposed; if old.address != c.address { - s.database_logins.clear_requests(); - resolve_project(s); + // Only the two databases this definition names can change: another database's layers + // waiting for a login keep their request, and an unrelated down server is not retried. + let mut keys = address_keys(&old.address); + keys.extend(address_keys(&c.address)); + resolve_keys(s, &keys); } if c.kind == "s3" && (old.name != c.name || old.address != c.address || old.options != c.options) { s3::disconnect(&old.name); @@ -212,8 +215,7 @@ fn duplicate(s: &mut Session, p: &Value) -> CmdResult { c.name = name.into(); } let (c, _) = s.connections.add(c, explicit)?; - s.database_logins.clear_requests(); - resolve_project(s); + resolve_keys(s, &address_keys(&c.address)); Ok(shown(s, &c)) } @@ -226,8 +228,9 @@ fn remove(s: &mut Session, p: &Value) -> CmdResult { clear_identity(s, &c.id); } s.connections = proposed; - s.database_logins.clear_requests(); - resolve_project(s); + // The removed definition named one database: only its layers can change. The requests naming + // it went with `clear_identity`. + resolve_keys(s, &address_keys(&c.address)); if c.kind == "s3" { s3::disconnect(&c.name); } @@ -252,6 +255,17 @@ fn clear_identity(s: &mut Session, id: &str) { rustgis_data::source::duckdb::forget_connection_password(id); } +/// The databases an address names (empty for kinds that read no database). +fn address_keys(address: &str) -> Vec<(&'static str, String)> { + crate::connections::database_key(address).into_iter().collect() +} + +/// The database a layer reads, as its driver names it (the same key `resolve_project` groups by). +pub(crate) fn layer_keys(src: &rustgis_map::source::SourceLayer) -> Vec<(&'static str, String)> { + let Some((_, database)) = crate::connections::layer_database(src) else { return Vec::new() }; + crate::connections::database_key(&crate::sources::redact(database)).into_iter().collect() +} + fn forget_password(s: &mut Session, p: &Value) -> CmdResult { let c = found(s, p)?.clone(); if !DATABASE_KINDS.contains(&c.kind.as_str()) { diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index bd4b7840f..1a6400c37 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -1893,7 +1893,9 @@ pub(crate) fn add_layer_text(s: &mut Session, text: &str) -> CmdResult { super::connections::clear_active(); s.project.normalize(); super::connections::restore_active(s); - super::connections::resolve_project(s); + // Only the database the added layer reads can change here. + let keys: Vec<_> = s.map().layer(id).and_then(|l| l.source.as_ref()).map(super::connections::layer_keys).unwrap_or_default(); + super::connections::resolve_keys(s, &keys); super::connections::note_waiting(s); s.active_layer = Some(id); Ok(json!({"layer": id})) diff --git a/crates/engine/src/tests/remembered_database_packets.rs b/crates/engine/src/tests/remembered_database_packets.rs index db9a3158a..d02fba07f 100644 --- a/crates/engine/src/tests/remembered_database_packets.rs +++ b/crates/engine/src/tests/remembered_database_packets.rs @@ -127,6 +127,7 @@ impl Server { worker: Some(worker), } } + #[track_caller] fn take(&self) -> Vec { self.packets.recv_timeout(std::time::Duration::from_secs(3)).unwrap() @@ -150,8 +151,13 @@ fn app(store: Arc, prefs: &str) -> Session { s } fn project(address: &str) -> String { + project_layers(address, 2) +} + +/// A project whose first `n` layers read `address`. +fn project_layers(address: &str, n: usize) -> String { let mut p = world().project; - for layer in p.map_mut().layers.iter_mut().take(2) { + for layer in p.map_mut().layers.iter_mut().take(n) { layer.source = Some(serde_json::from_value(json!({"postgis":{"database":address,"table":"roads","family":"Lines"}})).unwrap()); } serde_json::to_string(&p).unwrap() @@ -450,3 +456,35 @@ fn remembered_database_test_listing_failure_keeps_authenticated_session() { assert_eq!(s.database_logins.session_password(&b).unwrap().expose(), "prior-b"); server.empty(); } + +#[test] +fn remembered_database_incremental_change_resolves_only_its_database() { + if isolated("remembered_database_incremental_change_resolves_only_its_database") { + return; + } + let server = Server::start(true); + let store = Arc::new(MemorySecrets::new()); + let mut s = app(store, "{}"); + let id = remember(&mut s, "Waiting", &server.address, "waiting-sentinel"); + // One layer waits for this database. Opening the project connects once, and the catalogue + // read fails (not a refusal), so the group stays unopened for a later resolution. + s.run("project.openText", &json!({"text":project_layers(&server.address, 1)})).unwrap(); + assert_eq!(server.take(), b"waiting-sentinel\0"); + // A change to another connection must not connect to this database again. + s.run( + "connection.new", + &json!({"kind":"postgres","name":"Far","address":"postgres:host=127.0.0.1 port=1 user=far dbname=gis sslmode=disable"}), + ) + .unwrap(); + server.empty(); + // A change to this database's own definition still resolves its layers. + let same = s.run("connection.new", &json!({"kind":"postgres","name":"Same","address":server.address})).unwrap()["id"] + .as_str() + .unwrap() + .to_string(); + assert_eq!(server.take(), b"waiting-sentinel\0"); + s.run("connection.remove", &json!({"id":same})).unwrap(); + assert_eq!(server.take(), b"waiting-sentinel\0"); + server.empty(); + assert!(s.connections.find(&id).is_some()); +} diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index 21cff0061..c9cfcce61 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -508,7 +508,9 @@ prompt. Use Forget to delete a previously saved password. A database login reopens waiting layers at the same password-free address; `connection.list` reports them as `waiting`, and `connection.login` returns `reopened` and -`failed` counts. `connection.test` never persists a password. For a named connection, if the +`failed` counts. Adding, editing or removing a saved connection, or adding a layer, resolves +only the databases that change: an unrelated database that is down is not connected to again. +`connection.test` never persists a password. For a named connection, if the server positively refuses a password typed for that Test, the previous per-ID session credential, selection and active address password are restored (or the attempted credential is removed if none existed). From d4766fe59a425d506bd4398b507eb72d829c32f7 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Sat, 24 Oct 2026 03:32:08 +0200 Subject: [PATCH 4/8] Keep a refused login from taking the session and the address --- crates/engine/src/cmd/connections.rs | 20 +++++--- .../src/tests/remembered_database_packets.rs | 49 +++++++++++++++++++ docs/user-guide/data.md | 9 ++-- 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/cmd/connections.rs b/crates/engine/src/cmd/connections.rs index 5620c152a..cd66f1392 100644 --- a/crates/engine/src/cmd/connections.rs +++ b/crates/engine/src/cmd/connections.rs @@ -395,29 +395,37 @@ fn login(s: &mut Session, c: &Connection, p: &Value) -> Result #[cfg(not(target_arch = "wasm32"))] { let key = crate::connections::database_key(&c.address).ok_or("not a database connection")?; - rustgis_data::source::duckdb::set_connection_password(&key.1, &c.id, None); - s.database_logins.drop_activation(&key.1); s.database_logins.retry_saved(&c.id); let typed = ps(p, "password").filter(|t| !t.is_empty()); // A password typed for this address wins over any saved one (explicit input first). let address_typed = typed.is_none() && rustgis_data::source::duckdb::has_explicit_password(&key.1); let (password, saved) = match typed { - Some(pw) => { - s.database_logins.set_session_password(&c.id, pw); - (Some(rustgis_data::secrets::Secret::new(pw)), false) - } + Some(pw) => (Some(rustgis_data::secrets::Secret::new(pw)), false), None if address_typed => (None, false), None => { + // Resolved before this connection takes the address over: a login that turns + // out to have no password leaves another connection's activation alone. let (pw, saved) = credential(s, c)?.ok_or("password is required")?; (Some(pw), saved) } }; + // Another saved ID's activation is not authority to read this connection; a password + // typed for the address itself stays. + rustgis_data::source::duckdb::set_connection_password(&key.1, &c.id, None); + s.database_logins.drop_activation(&key.1); let all_unopened = s.database_logins.requests(s.opened).any(|r| r.kind == key.0 && r.key == key.1); s.database_logins.select(key.0, &key.1, &c.id); if let Some(password) = &password { activate(s, c, password.expose()); } let mut result = reopen(s, c, saved, all_unopened); + if let Some(pw) = &typed + && !result.refused + { + // A refused password is not this session's credential: it was sent, and the user + // is asked for another one. + s.database_logins.set_session_password(&c.id, pw); + } if remember { let stored = !result.refused && typed.is_some_and(|pw| s.database_logins.save(c, pw)); if stored { diff --git a/crates/engine/src/tests/remembered_database_packets.rs b/crates/engine/src/tests/remembered_database_packets.rs index d02fba07f..c964e08e3 100644 --- a/crates/engine/src/tests/remembered_database_packets.rs +++ b/crates/engine/src/tests/remembered_database_packets.rs @@ -488,3 +488,52 @@ fn remembered_database_incremental_change_resolves_only_its_database() { server.empty(); assert!(s.connections.find(&id).is_some()); } + +#[test] +fn remembered_database_refused_login_does_not_keep_the_typed_password() { + if isolated("remembered_database_refused_login_does_not_keep_the_typed_password") { + return; + } + let server = Server::new(); + let store = Arc::new(MemorySecrets::new()); + let mut seed = app(store.clone(), "{}"); + let id = remember(&mut seed, "Refused", &server.address, "saved-a"); + let mut s = app(store.clone(), &seed.connections.to_json()); + // The saved password is refused, so the layer waits and the login below reaches the server. + s.run("project.openText", &json!({"text":project_layers(&server.address, 1)})).unwrap(); + assert_eq!(server.take(), b"saved-a\0"); + assert!(s.database_logins.is_rejected(&id)); + // A password typed for the login is sent, but a refusal does not make it this session's. + let result = s.run("connection.login", &json!({"id":id,"password":"typed-wrong"})).unwrap(); + assert_eq!(result["failed"], 1, "{result}"); + assert_eq!(server.take(), b"typed-wrong\0"); + assert!(!s.database_logins.has_session_password(&id)); + assert!(store.load(&format!("connection/{id}")).unwrap().is_some()); + server.empty(); +} + +#[test] +fn remembered_database_passwordless_login_keeps_another_connection_active() { + if isolated("remembered_database_passwordless_login_keeps_another_connection_active") { + return; + } + use rustgis_data::source::duckdb; + let server = Server::new(); + let store = Arc::new(MemorySecrets::new()); + let mut s = app(store, "{}"); + let a = remember(&mut s, "A", &server.address, "owner-a"); + let b = s.run("connection.new", &json!({"kind":"postgres","name":"B","address":server.address})).unwrap()["id"] + .as_str() + .unwrap() + .to_string(); + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + s.run("connection.login", &json!({"id":a})).unwrap(); + assert!(duckdb::has_connection_password(&key, &a)); + // A login for a connection with no password of its own fails without taking the address's + // activation away from the connection that is using it. + let error = s.run("connection.login", &json!({"id":b})).unwrap_err(); + assert!(error.contains("password is required"), "{error}"); + assert!(duckdb::has_connection_password(&key, &a)); + assert!(s.database_logins.activations().contains(&(key.clone(), a.clone()))); + server.empty(); +} diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index c9cfcce61..c9263b661 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -501,10 +501,11 @@ takes precedence over existing ambient fallbacks on explicitly initiated connect Opening a project never newly authorizes environment/password-file credentials or Windows identity. A positively refused saved password is not retried for the rest of the session, and no other saved password is tried automatically. Its old OS-store value is retained. -The fresh login form has an empty password and Remember unchecked: an unchecked replacement -is session-only and does not overwrite or delete the old value; checking Remember and -successfully saving replaces it. A transport error or missing table is not a wrong-password -prompt. Use Forget to delete a previously saved password. +The fresh login form has an empty password and Remember unchecked: an unchecked replacement is +session-only and does not overwrite or delete the old value; checking Remember and successfully +saving replaces it. A password the server refuses is not kept as this session's. A transport +error or missing table is not a wrong-password prompt. Use Forget to delete a previously saved +password. A database login reopens waiting layers at the same password-free address; `connection.list` reports them as `waiting`, and `connection.login` returns `reopened` and From ecef2ab8390ed76abd8b8ae9ea97eb85242066a8 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Sat, 24 Oct 2026 03:43:54 +0200 Subject: [PATCH 5/8] Attempt a refused login once per database --- crates/engine/src/cmd/connections.rs | 4 +++- crates/engine/src/tests/remembered_database_packets.rs | 6 +++--- docs/user-guide/data.md | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/connections.rs b/crates/engine/src/cmd/connections.rs index cd66f1392..cdbc331d2 100644 --- a/crates/engine/src/cmd/connections.rs +++ b/crates/engine/src/cmd/connections.rs @@ -319,7 +319,9 @@ fn reopen(s: &mut Session, c: &Connection, saved: bool, all_unopened: bool) -> R if kind != key.0 || crate::connections::database_key(&crate::sources::redact(database)).as_ref() != Some(&key) { continue; } - if saved && result.refused { + if result.refused { + // The server has already refused this attempt's password: the group's other layers + // must not send it again (one attempt per database, typed or saved). src.login = true; src.error = first_error.clone(); result.failed += 1; diff --git a/crates/engine/src/tests/remembered_database_packets.rs b/crates/engine/src/tests/remembered_database_packets.rs index c964e08e3..552500747 100644 --- a/crates/engine/src/tests/remembered_database_packets.rs +++ b/crates/engine/src/tests/remembered_database_packets.rs @@ -225,8 +225,9 @@ fn remembered_database_unique_packet_refusal_and_replacement() { s.run("project.openText", &json!({"text":text})).unwrap(); server.empty(); s.run("connection.login", &json!({"id":id,"password":"unchecked-new"})).unwrap(); + // One attempt per database: the refused password is not sent to the group's other layers. assert_eq!(server.take(), b"unchecked-new\0"); - assert_eq!(server.take(), b"unchecked-new\0"); + server.empty(); assert_eq!(store.load(&format!("connection/{id}")).unwrap(), saved); assert!(s.database_logins.is_rejected(&id)); // A refused password is not "available for this session", and the store keeps the old value. @@ -234,7 +235,7 @@ fn remembered_database_unique_packet_refusal_and_replacement() { assert_eq!(refused["passwordSaved"], false, "{refused}"); assert!(refused.get("warning").is_none(), "a refused password must not be called available: {refused}"); assert_eq!(server.take(), b"refused-with-remember\0"); - assert_eq!(server.take(), b"refused-with-remember\0"); + server.empty(); assert_eq!(store.load(&format!("connection/{id}")).unwrap(), saved); assert!(s.database_logins.is_rejected(&id)); s.run("project.new", &Value::Null).unwrap(); @@ -306,7 +307,6 @@ fn remembered_database_exact_address_and_explicit_precedence_packets() { // saved one either: the address's wins, as it does when the project opens. explicit.run("connection.login", &json!({"id":id})).unwrap(); assert_eq!(server.take(), b"explicit-sentinel\0"); - assert_eq!(server.take(), b"explicit-sentinel\0"); server.empty(); } diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index c9263b661..19f02c74e 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -503,9 +503,9 @@ identity. A positively refused saved password is not retried for the rest of the and no other saved password is tried automatically. Its old OS-store value is retained. The fresh login form has an empty password and Remember unchecked: an unchecked replacement is session-only and does not overwrite or delete the old value; checking Remember and successfully -saving replaces it. A password the server refuses is not kept as this session's. A transport -error or missing table is not a wrong-password prompt. Use Forget to delete a previously saved -password. +saving replaces it. A password the server refuses is not kept as this session's, and is not sent +to the database's other layers. A transport error or missing table is not a wrong-password +prompt. Use Forget to delete a previously saved password. A database login reopens waiting layers at the same password-free address; `connection.list` reports them as `waiting`, and `connection.login` returns `reopened` and From b5ab0908cc290c6738d7e84043ffe2d07c57c69d Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Sat, 24 Oct 2026 10:17:40 +0200 Subject: [PATCH 6/8] Restore active credentials after refused typed login --- crates/data/src/source/duckdb.rs | 6 +- crates/engine/src/cmd/connection_use.rs | 2 +- crates/engine/src/cmd/connections.rs | 37 ++++++++---- .../src/tests/remembered_database_packets.rs | 58 +++++++++++++++++++ docs/user-guide/data.md | 6 +- 5 files changed, 92 insertions(+), 17 deletions(-) diff --git a/crates/data/src/source/duckdb.rs b/crates/data/src/source/duckdb.rs index 24b1264d8..5f7337ae7 100644 --- a/crates/data/src/source/duckdb.rs +++ b/crates/data/src/source/duckdb.rs @@ -175,12 +175,12 @@ pub fn has_connection_password(dsn: &str, id: &str) -> bool { } /// Tries an explicitly supplied password, restoring the previous address slot and its ownership -/// only when the server refuses authentication. Other failures (including table listing) retain +/// when `refused` classifies the outcome as an authentication refusal. Other failures retain /// the new password. Never holds the cache lock during network I/O. -pub fn try_connection_password(dsn: &str, id: &str, password: &str, run: impl FnOnce() -> Result) -> Result { +pub fn try_connection_password(dsn: &str, id: &str, password: &str, run: impl FnOnce() -> T, refused: impl FnOnce(&T) -> bool) -> T { let previous = lock().insert(dsn.to_string(), Entry { password: Secret::new(password), connection_id: Some(id.to_string()) }); let result = run(); - if matches!(&result, Err(DataError::Login(_))) { + if refused(&result) { let mut all = lock(); if let Some(previous) = previous { all.insert(dsn.to_string(), previous); diff --git a/crates/engine/src/cmd/connection_use.rs b/crates/engine/src/cmd/connection_use.rs index 7772bcce9..e31ef323e 100644 --- a/crates/engine/src/cmd/connection_use.rs +++ b/crates/engine/src/cmd/connection_use.rs @@ -54,7 +54,7 @@ pub(super) fn tables(s: &mut Session, c: &Connection, typed: Option<&str>) -> Re s.database_logins.select(kind, key, &c.id); } let result = if let Some(pw) = typed { - duckdb::try_connection_password(key, &c.id, pw, || read_tables(&database)) + duckdb::try_connection_password(key, &c.id, pw, || read_tables(&database), |result| matches!(result, Err(DataError::Login(_)))) } else { read_tables(&database) }; diff --git a/crates/engine/src/cmd/connections.rs b/crates/engine/src/cmd/connections.rs index cdbc331d2..3c37ae98f 100644 --- a/crates/engine/src/cmd/connections.rs +++ b/crates/engine/src/cmd/connections.rs @@ -397,12 +397,14 @@ fn login(s: &mut Session, c: &Connection, p: &Value) -> Result #[cfg(not(target_arch = "wasm32"))] { let key = crate::connections::database_key(&c.address).ok_or("not a database connection")?; - s.database_logins.retry_saved(&c.id); let typed = ps(p, "password").filter(|t| !t.is_empty()); + if typed.is_none() { + s.database_logins.retry_saved(&c.id); + } // A password typed for this address wins over any saved one (explicit input first). let address_typed = typed.is_none() && rustgis_data::source::duckdb::has_explicit_password(&key.1); let (password, saved) = match typed { - Some(pw) => (Some(rustgis_data::secrets::Secret::new(pw)), false), + Some(_) => (None, false), None if address_typed => (None, false), None => { // Resolved before this connection takes the address over: a login that turns @@ -411,22 +413,35 @@ fn login(s: &mut Session, c: &Connection, p: &Value) -> Result (Some(pw), saved) } }; - // Another saved ID's activation is not authority to read this connection; a password - // typed for the address itself stays. - rustgis_data::source::duckdb::set_connection_password(&key.1, &c.id, None); - s.database_logins.drop_activation(&key.1); let all_unopened = s.database_logins.requests(s.opened).any(|r| r.kind == key.0 && r.key == key.1); - s.database_logins.select(key.0, &key.1, &c.id); - if let Some(password) = &password { - activate(s, c, password.expose()); - } - let mut result = reopen(s, c, saved, all_unopened); + let mut result = if let Some(pw) = typed { + // Keep selection and activation unchanged until this attempt is accepted. + rustgis_data::source::duckdb::try_connection_password( + &key.1, + &c.id, + pw, + || reopen(s, c, false, all_unopened), + |result| result.refused, + ) + } else { + // Another saved ID's activation is not authority to read this connection; + // a password typed for the address itself stays. + rustgis_data::source::duckdb::set_connection_password(&key.1, &c.id, None); + s.database_logins.drop_activation(&key.1); + s.database_logins.select(key.0, &key.1, &c.id); + if let Some(password) = &password { + activate(s, c, password.expose()); + } + reopen(s, c, saved, all_unopened) + }; if let Some(pw) = &typed && !result.refused { // A refused password is not this session's credential: it was sent, and the user // is asked for another one. s.database_logins.set_session_password(&c.id, pw); + s.database_logins.select(key.0, &key.1, &c.id); + s.database_logins.note_activation(&key.1, &c.id); } if remember { let stored = !result.refused && typed.is_some_and(|pw| s.database_logins.save(c, pw)); diff --git a/crates/engine/src/tests/remembered_database_packets.rs b/crates/engine/src/tests/remembered_database_packets.rs index 552500747..e69cbb99a 100644 --- a/crates/engine/src/tests/remembered_database_packets.rs +++ b/crates/engine/src/tests/remembered_database_packets.rs @@ -503,11 +503,17 @@ fn remembered_database_refused_login_does_not_keep_the_typed_password() { s.run("project.openText", &json!({"text":project_layers(&server.address, 1)})).unwrap(); assert_eq!(server.take(), b"saved-a\0"); assert!(s.database_logins.is_rejected(&id)); + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + let selection = s.database_logins.selection("postgres", &key).map(str::to_string); + let activations = s.database_logins.activations(); // A password typed for the login is sent, but a refusal does not make it this session's. let result = s.run("connection.login", &json!({"id":id,"password":"typed-wrong"})).unwrap(); assert_eq!(result["failed"], 1, "{result}"); assert_eq!(server.take(), b"typed-wrong\0"); assert!(!s.database_logins.has_session_password(&id)); + assert!(!rustgis_data::source::duckdb::has_password(&key)); + assert_eq!(s.database_logins.selection("postgres", &key), selection.as_deref()); + assert_eq!(s.database_logins.activations(), activations); assert!(store.load(&format!("connection/{id}")).unwrap().is_some()); server.empty(); } @@ -537,3 +543,55 @@ fn remembered_database_passwordless_login_keeps_another_connection_active() { assert!(s.database_logins.activations().contains(&(key.clone(), a.clone()))); server.empty(); } + +#[test] +fn remembered_database_refused_login_restores_previous_driver_owner() { + if isolated("remembered_database_refused_login_restores_previous_driver_owner") { + return; + } + use rustgis_data::source::{duckdb, postgis}; + let server = Server::new(); + let store = Arc::new(MemorySecrets::new()); + let mut s = app(store.clone(), "{}"); + let a = remember(&mut s, "A", &server.address, "prior-a"); + let b = remember(&mut s, "B", &server.address, "prior-b"); + // Stage two waiting layers without letting a project open change the active credential. + s.project = serde_json::from_str(&project(&server.address)).unwrap(); + for layer in s.map_mut().layers.iter_mut().take(2) { + layer.source.as_mut().unwrap().login = true; + } + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + let saved = store.load(&format!("connection/{a}")).unwrap(); + for (owner, expected) in [(Some(b.as_str()), "prior-b"), (Some(a.as_str()), "prior-a"), (None, "prior-address")] { + if let Some(owner) = owner { + duckdb::set_connection_password(&key, owner, Some(expected)); + s.database_logins.select("postgres", &key, owner); + s.database_logins.note_activation(&key, owner); + } else { + duckdb::remember_password(&key, expected); + } + let selection = s.database_logins.selection("postgres", &key).map(str::to_string); + let activations = s.database_logins.activations(); + let result = s.run("connection.login", &json!({"id":a,"password":"login-refused","remember":true})).unwrap(); + assert_eq!(result["failed"], 2); + assert_eq!(result["passwordSaved"], false); + assert_eq!(server.take(), b"login-refused\0"); + server.empty(); + assert_eq!(s.database_logins.session_password(&a).unwrap().expose(), "prior-a"); + assert_eq!(s.database_logins.selection("postgres", &key), selection.as_deref()); + assert_eq!(s.database_logins.activations(), activations); + assert_eq!(duckdb::has_explicit_password(&key), owner.is_none()); + if let Some(owner) = owner { + assert!(duckdb::has_connection_password(&key, owner)); + } + let listing = s.run("connection.list", &Value::Null).unwrap(); + assert_eq!(listing["loginRequests"][0]["reason"], "password"); + assert_eq!(listing["loginRequests"][0]["connections"], json!([a])); + assert!(postgis::connect(&key, false).is_err()); + let mut packet = expected.as_bytes().to_vec(); + packet.push(0); + assert_eq!(server.take(), packet); + assert_eq!(store.load(&format!("connection/{a}")).unwrap(), saved); + } + server.empty(); +} diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index 19f02c74e..eeca4e51c 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -504,8 +504,10 @@ and no other saved password is tried automatically. Its old OS-store value is re The fresh login form has an empty password and Remember unchecked: an unchecked replacement is session-only and does not overwrite or delete the old value; checking Remember and successfully saving replaces it. A password the server refuses is not kept as this session's, and is not sent -to the database's other layers. A transport error or missing table is not a wrong-password -prompt. Use Forget to delete a previously saved password. +to the database's other layers. A refused typed login restores the previous active password, +selection and activation; with no previous password, the attempted value is removed. +A transport error or missing table is not a wrong-password prompt. Use Forget to delete a +previously saved password. A database login reopens waiting layers at the same password-free address; `connection.list` reports them as `waiting`, and `connection.login` returns `reopened` and From 69abea7954a8a6b8b74c77e67b0616582cf3b460 Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Sat, 24 Oct 2026 11:27:49 +0200 Subject: [PATCH 7/8] Evict cached session credentials on authentication refusal --- crates/engine/src/cmd/connection_logins.rs | 2 +- crates/engine/src/cmd/connection_use.rs | 5 ++ crates/engine/src/cmd/connections.rs | 11 ++- crates/engine/src/connections.rs | 6 ++ .../src/tests/remembered_database_packets.rs | 80 +++++++++++++++++++ docs/user-guide/data.md | 3 + 6 files changed, 103 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/connection_logins.rs b/crates/engine/src/cmd/connection_logins.rs index e867e5849..b69012f2e 100644 --- a/crates/engine/src/cmd/connection_logins.rs +++ b/crates/engine/src/cmd/connection_logins.rs @@ -112,7 +112,7 @@ fn resolve(s: &mut Session, only: Option<&[(&'static str, String)]>) { match super::credential(s, &c) { Ok(Some((pw, saved))) => { super::activate(s, &c, pw.expose()); - super::reopen(s, &c, saved, true); + super::reopen(s, &c, saved, !saved, true); } Ok(None) => { duckdb::set_connection_password(&key, &c.id, None); diff --git a/crates/engine/src/cmd/connection_use.rs b/crates/engine/src/cmd/connection_use.rs index e31ef323e..61e93ec4a 100644 --- a/crates/engine/src/cmd/connection_use.rs +++ b/crates/engine/src/cmd/connection_use.rs @@ -47,6 +47,7 @@ pub(super) fn tables(s: &mut Session, c: &Connection, typed: Option<&str>) -> Re return Err("password is required".into()); } let saved = credential.as_ref().is_some_and(|(_, saved)| *saved); + let session = credential.as_ref().is_some_and(|(_, saved)| !*saved); if typed.is_none() && let Some((password, _)) = &credential { @@ -77,6 +78,10 @@ pub(super) fn tables(s: &mut Session, c: &Connection, typed: Option<&str>) -> Re if matches!(&error, DataError::Login(_)) { if saved { s.database_logins.reject(&c.id); + } else if session { + s.database_logins.forget_session_password(&c.id); + } + if saved || session { duckdb::forget_connection_password(&c.id); s.database_logins.drop_activation(key); } diff --git a/crates/engine/src/cmd/connections.rs b/crates/engine/src/cmd/connections.rs index 3c37ae98f..39fd8700c 100644 --- a/crates/engine/src/cmd/connections.rs +++ b/crates/engine/src/cmd/connections.rs @@ -309,7 +309,7 @@ struct Reopened { } #[cfg(not(target_arch = "wasm32"))] -fn reopen(s: &mut Session, c: &Connection, saved: bool, all_unopened: bool) -> Reopened { +fn reopen(s: &mut Session, c: &Connection, saved: bool, session: bool, all_unopened: bool) -> Reopened { let Some(key) = crate::connections::database_key(&c.address) else { return Reopened::default() }; let mut result = Reopened::default(); let mut first_error = None; @@ -341,6 +341,10 @@ fn reopen(s: &mut Session, c: &Connection, saved: bool, all_unopened: bool) -> R use crate::connections::{LoginReason, LoginRequest}; if saved { s.database_logins.reject(&c.id); + } else if session { + s.database_logins.forget_session_password(&c.id); + } + if saved || session { rustgis_data::source::duckdb::forget_connection_password(&c.id); s.database_logins.drop_activation(&key.1); } @@ -420,7 +424,7 @@ fn login(s: &mut Session, c: &Connection, p: &Value) -> Result &key.1, &c.id, pw, - || reopen(s, c, false, all_unopened), + || reopen(s, c, false, false, all_unopened), |result| result.refused, ) } else { @@ -432,7 +436,8 @@ fn login(s: &mut Session, c: &Connection, p: &Value) -> Result if let Some(password) = &password { activate(s, c, password.expose()); } - reopen(s, c, saved, all_unopened) + let session = password.is_some() && !saved; + reopen(s, c, saved, session, all_unopened) }; if let Some(pw) = &typed && !result.refused diff --git a/crates/engine/src/connections.rs b/crates/engine/src/connections.rs index 24eceeb56..e6d59c555 100644 --- a/crates/engine/src/connections.rs +++ b/crates/engine/src/connections.rs @@ -526,6 +526,12 @@ impl DatabaseLogins { self.session.insert(id.to_string(), Secret::new(password)); } + /// Drops only this ID's session password after the server refused that cached credential. + #[cfg(not(target_arch = "wasm32"))] + pub(crate) fn forget_session_password(&mut self, id: &str) { + self.session.remove(id); + } + /// Reads the password saved for a definition (once: later calls use what was read). pub fn load(&mut self, c: &Connection) -> SavedPassword { if let Some(p) = self.saved.get(&c.id) { diff --git a/crates/engine/src/tests/remembered_database_packets.rs b/crates/engine/src/tests/remembered_database_packets.rs index e69cbb99a..aefc3b6c3 100644 --- a/crates/engine/src/tests/remembered_database_packets.rs +++ b/crates/engine/src/tests/remembered_database_packets.rs @@ -595,3 +595,83 @@ fn remembered_database_refused_login_restores_previous_driver_owner() { } server.empty(); } + +#[test] +fn remembered_database_cached_session_refusal_evicts_reused_password() { + if isolated("remembered_database_cached_session_refusal_evicts_reused_password") { + return; + } + use rustgis_data::source::duckdb; + let server = Server::new(); + for command in ["connection.login", "connection.test", "connection.browse", "project.openText"] { + let store = Arc::new(MemorySecrets::new()); + let mut s = app(store.clone(), "{}"); + let id = s.run("connection.new", &json!({"kind":"postgres","name":"Session","address":server.address})).unwrap()["id"] + .as_str() + .unwrap() + .to_string(); + s.run("connection.login", &json!({"id":id,"password":"cached-refused"})).unwrap(); + let text = project(&server.address); + s.project = serde_json::from_str(&text).unwrap(); + for layer in s.map_mut().layers.iter_mut().take(2) { + layer.source.as_mut().unwrap().login = true; + } + let params = if command == "project.openText" { json!({"text":text}) } else { json!({"id":id}) }; + let result = s.run(command, ¶ms); + match command { + "connection.login" => assert_eq!(result.unwrap()["failed"], 2), + "project.openText" => assert!(result.is_ok()), + _ => assert!(!result.unwrap_err().contains("cached-refused")), + } + assert_eq!(server.take(), b"cached-refused\0", "{command}"); + server.empty(); + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + assert!(!s.database_logins.has_session_password(&id), "{command}"); + assert!(!duckdb::has_password(&key), "{command}"); + assert!(!s.database_logins.activations().iter().any(|(active, _)| active == &key), "{command}"); + assert!(!s.database_logins.is_rejected(&id), "a session refusal must not reject the saved credential"); + assert!(store.load(&format!("connection/{id}")).unwrap().is_none()); + let listing = s.run("connection.list", &Value::Null).unwrap(); + assert_eq!(listing["connections"][0]["loggedIn"], false); + assert_eq!(listing["loginRequests"][0]["reason"], "password"); + assert_eq!(listing["loginRequests"][0]["connections"], json!([id])); + // A second password-free operation must not put the refused password on the wire again. + let _ = s.run(command, ¶ms); + server.empty(); + } +} + +#[test] +fn remembered_database_cached_session_listing_failure_keeps_password() { + if isolated("remembered_database_cached_session_listing_failure_keeps_password") { + return; + } + use rustgis_data::source::duckdb; + let server = Server::start(true); + for command in ["connection.login", "connection.test", "connection.browse", "project.openText"] { + let mut s = app(Arc::new(MemorySecrets::new()), "{}"); + let id = s.run("connection.new", &json!({"kind":"postgres","name":"Session","address":server.address})).unwrap()["id"] + .as_str() + .unwrap() + .to_string(); + s.run("connection.login", &json!({"id":id,"password":"cached-accepted"})).unwrap(); + let text = project_layers(&server.address, 1); + s.project = serde_json::from_str(&text).unwrap(); + s.map_mut().layers[0].source.as_mut().unwrap().login = true; + let params = if command == "project.openText" { json!({"text":text}) } else { json!({"id":id}) }; + let result = s.run(command, ¶ms); + match command { + "connection.login" => assert_eq!(result.unwrap()["failed"], 1), + "project.openText" => assert!(result.is_ok()), + _ => assert!(result.unwrap_err().contains("fixture listing failed")), + } + assert_eq!(server.take(), b"cached-accepted\0", "{command}"); + server.empty(); + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + assert_eq!(s.database_logins.session_password(&id).unwrap().expose(), "cached-accepted", "{command}"); + assert!(duckdb::has_connection_password(&key, &id), "{command}"); + assert!(s.database_logins.activations().contains(&(key, id.clone())), "{command}"); + assert!(!s.database_logins.is_rejected(&id)); + assert!(s.database_logins.requests(s.opened).next().is_none(), "{command}"); + } +} diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index eeca4e51c..d5350e011 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -506,6 +506,9 @@ session-only and does not overwrite or delete the old value; checking Remember a saving replaces it. A password the server refuses is not kept as this session's, and is not sent to the database's other layers. A refused typed login restores the previous active password, selection and activation; with no previous password, the attempted value is removed. +If a password reused from this ID's session cache is refused, that cached value, its owned +driver entry and activation are cleared, and a password is requested. This does not reject +or delete the ID's OS-store password. A transport error or missing table is not a wrong-password prompt. Use Forget to delete a previously saved password. From 44c1143b85fab9962ebc18f9e668bfd3cf7f78ab Mon Sep 17 00:00:00 2001 From: Daniel Hofbauer Date: Sat, 24 Oct 2026 12:03:07 +0200 Subject: [PATCH 8/8] Suppress equivalent saved copies of refused session passwords --- crates/engine/src/cmd/connection_use.rs | 2 +- crates/engine/src/cmd/connections.rs | 2 +- crates/engine/src/connections.rs | 18 +++++-- .../src/tests/remembered_database_packets.rs | 54 +++++++++++++++++++ docs/user-guide/data.md | 5 +- 5 files changed, 74 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/connection_use.rs b/crates/engine/src/cmd/connection_use.rs index 61e93ec4a..7cde29cc5 100644 --- a/crates/engine/src/cmd/connection_use.rs +++ b/crates/engine/src/cmd/connection_use.rs @@ -79,7 +79,7 @@ pub(super) fn tables(s: &mut Session, c: &Connection, typed: Option<&str>) -> Re if saved { s.database_logins.reject(&c.id); } else if session { - s.database_logins.forget_session_password(&c.id); + s.database_logins.reject_session_password(c); } if saved || session { duckdb::forget_connection_password(&c.id); diff --git a/crates/engine/src/cmd/connections.rs b/crates/engine/src/cmd/connections.rs index 39fd8700c..0c312711d 100644 --- a/crates/engine/src/cmd/connections.rs +++ b/crates/engine/src/cmd/connections.rs @@ -342,7 +342,7 @@ fn reopen(s: &mut Session, c: &Connection, saved: bool, session: bool, all_unope if saved { s.database_logins.reject(&c.id); } else if session { - s.database_logins.forget_session_password(&c.id); + s.database_logins.reject_session_password(c); } if saved || session { rustgis_data::source::duckdb::forget_connection_password(&c.id); diff --git a/crates/engine/src/connections.rs b/crates/engine/src/connections.rs index e6d59c555..9c0ce48c7 100644 --- a/crates/engine/src/connections.rs +++ b/crates/engine/src/connections.rs @@ -526,10 +526,22 @@ impl DatabaseLogins { self.session.insert(id.to_string(), Secret::new(password)); } - /// Drops only this ID's session password after the server refused that cached credential. + /// Drops a refused session credential and suppresses an equivalent saved value for this run. #[cfg(not(target_arch = "wasm32"))] - pub(crate) fn forget_session_password(&mut self, id: &str) { - self.session.remove(id); + pub(crate) fn reject_session_password(&mut self, c: &Connection) { + let Some(refused) = self.session.remove(&c.id) else { return }; + if !c.remembered || self.is_rejected(&c.id) { + return; + } + let same = match self.saved.get(&c.id) { + Some(saved) => saved == &refused, + // Explicit login/Refresh may have invalidated the lazy cache. Validate the bound + // account through the existing lookup before deciding whether it is the same value. + None => matches!(self.load(c), SavedPassword::Found(saved) if saved == refused), + }; + if same { + self.reject(&c.id); + } } /// Reads the password saved for a definition (once: later calls use what was read). diff --git a/crates/engine/src/tests/remembered_database_packets.rs b/crates/engine/src/tests/remembered_database_packets.rs index aefc3b6c3..5c0ee3ebe 100644 --- a/crates/engine/src/tests/remembered_database_packets.rs +++ b/crates/engine/src/tests/remembered_database_packets.rs @@ -675,3 +675,57 @@ fn remembered_database_cached_session_listing_failure_keeps_password() { assert!(s.database_logins.requests(s.opened).next().is_none(), "{command}"); } } + +#[test] +fn remembered_database_session_refusal_suppresses_only_matching_saved_value() { + if isolated("remembered_database_session_refusal_suppresses_only_matching_saved_value") { + return; + } + use rustgis_data::source::duckdb; + let server = Server::new(); + for same in [true, false] { + for command in ["connection.login", "connection.test", "connection.browse", "project.openText"] { + let store = Arc::new(MemorySecrets::new()); + let mut s = app(store.clone(), "{}"); + let id = remember(&mut s, "Remembered", &server.address, "stored-value"); + let attempted = if same { "stored-value" } else { "session-other" }; + if !same { + s.run("connection.login", &json!({"id":id,"password":attempted})).unwrap(); + } + let saved = store.load(&format!("connection/{id}")).unwrap(); + let text = project(&server.address); + s.project = serde_json::from_str(&text).unwrap(); + for layer in s.map_mut().layers.iter_mut().take(2) { + layer.source.as_mut().unwrap().login = true; + } + // Passwordless login invalidates the saved cache before using the session value; + // the other entry points exercise the already-cached saved copy. + let params = if command == "project.openText" { json!({"text":text}) } else { json!({"id":id}) }; + let result = s.run(command, ¶ms); + match command { + "connection.login" => assert_eq!(result.unwrap()["failed"], 2), + "project.openText" => assert!(result.is_ok()), + _ => assert!(!result.unwrap_err().contains(attempted)), + } + let mut packet = attempted.as_bytes().to_vec(); + packet.push(0); + assert_eq!(server.take(), packet, "{command}, same={same}"); + server.empty(); + let (_, key) = crate::connections::database_key(&server.address).unwrap(); + assert!(!s.database_logins.has_session_password(&id)); + assert!(!duckdb::has_password(&key)); + assert_eq!(s.database_logins.is_rejected(&id), same, "{command}"); + assert!(s.connections.find(&id).unwrap().remembered); + assert_eq!(store.load(&format!("connection/{id}")).unwrap(), saved); + // Automatic resolution must not retry the same refused bytes via the saved copy. + s.run("project.openText", &json!({"text":text})).unwrap(); + if !same { + // A different saved value was not refused by the first attempt and remains eligible. + assert_eq!(server.take(), b"stored-value\0", "{command}"); + } + server.empty(); + assert_eq!(s.run("connection.list", &Value::Null).unwrap()["loginRequests"][0]["reason"], "password"); + assert_eq!(store.load(&format!("connection/{id}")).unwrap(), saved); + } + } +} diff --git a/docs/user-guide/data.md b/docs/user-guide/data.md index d5350e011..e85d75e71 100644 --- a/docs/user-guide/data.md +++ b/docs/user-guide/data.md @@ -507,8 +507,9 @@ saving replaces it. A password the server refuses is not kept as this session's, to the database's other layers. A refused typed login restores the previous active password, selection and activation; with no previous password, the attempted value is removed. If a password reused from this ID's session cache is refused, that cached value, its owned -driver entry and activation are cleared, and a password is requested. This does not reject -or delete the ID's OS-store password. +driver entry and activation are cleared, and a password is requested. An identical saved value +is also suppressed for this session; a different saved value is not marked refused by this +attempt. The OS-store entry is retained in either case. A transport error or missing table is not a wrong-password prompt. Use Forget to delete a previously saved password.