diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 4a937184b..6a9cfa6bf 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -38,7 +38,10 @@ jobs: # ubuntu:25.10 is minimal — git for Swatinem/rust-cache key # generation, curl + ca-certificates for rust-toolchain's # rustup install, plus the GTK / libadwaita / sourceview / - # OpenSSL / libsecret -dev packages the workspace links. + # OpenSSL / libsecret -dev packages the workspace links, plus + # libkrb5-dev + clang for the MSSQL driver's integrated + # (Kerberos/GSSAPI) auth (libgssapi-sys links gssapi_krb5 and + # runs bindgen). # No sudo (container runs as root by default). run: | apt-get update @@ -52,7 +55,9 @@ jobs: libadwaita-1-dev \ libgtksourceview-5-dev \ libssl-dev \ - libsecret-1-dev + libsecret-1-dev \ + libkrb5-dev \ + clang - uses: dtolnay/rust-toolchain@1.93 with: components: rustfmt, clippy @@ -65,8 +70,10 @@ jobs: run: cargo clippy --all-targets -- -D warnings - name: Build run: cargo build --workspace + # --bins matters: tablepro-app has no lib target, so --lib alone + # skips every test in the app crate. - name: Unit tests - run: cargo test --workspace --lib + run: cargo test --workspace --lib --bins integration: name: Driver integration tests (docker) diff --git a/linux/Cargo.lock b/linux/Cargo.lock index c79466419..9e23880ea 100644 --- a/linux/Cargo.lock +++ b/linux/Cargo.lock @@ -336,6 +336,26 @@ dependencies = [ "sha2", ] +[[package]] +name = "bindgen" +version = "0.71.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f58bf3d7db68cfbac37cfc485a8d711e87e064c3d0fe0435b92f7a407f9d6b3" +dependencies = [ + "bitflags", + "cexpr", + "clang-sys", + "itertools 0.13.0", + "log", + "prettyplease", + "proc-macro2", + "quote", + "regex", + "rustc-hash", + "shlex", + "syn 2.0.117", +] + [[package]] name = "bitflags" version = "2.11.1" @@ -600,6 +620,15 @@ dependencies = [ "shlex", ] +[[package]] +name = "cexpr" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6fac387a98bb7c37292057cffc56d62ecb629900026402633ae9160df93a8766" +dependencies = [ + "nom", +] + [[package]] name = "cfg-expr" version = "0.20.7" @@ -669,6 +698,17 @@ dependencies = [ "zeroize", ] +[[package]] +name = "clang-sys" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b023947811758c97c59bf9d1c188fd619ad4718dcaa767947df1cadb14f39f4" +dependencies = [ + "glob", + "libc", + "libloading", +] + [[package]] name = "clickhouse" version = "0.15.1" @@ -1672,6 +1712,12 @@ dependencies = [ "system-deps", ] +[[package]] +name = "glob" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e4eba85ea1d0a966a983acd07deee566e67395d2d96b6fb39e62b5a833f1eb0b" + [[package]] name = "gobject-sys" version = "0.22.0" @@ -2302,6 +2348,15 @@ version = "2.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d98f6fed1fde3f8c21bc40a1abb88dd75e67924f9cffc3ef95607bad8017f8e2" +[[package]] +name = "itertools" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" +dependencies = [ + "either", +] + [[package]] name = "itertools" version = "0.14.0" @@ -2457,6 +2512,38 @@ dependencies = [ "rand 0.9.4", ] +[[package]] +name = "libgssapi" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e668df13f2e97f3eed52d9301f6b1c4c1ccfccc30eab9e6628e4a8c1fc3546" +dependencies = [ + "bitflags", + "bytes", + "lazy_static", + "libgssapi-sys", +] + +[[package]] +name = "libgssapi-sys" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5103ac4557eacd36ff678b654b943f8966d3db9688fbd180a0b4c5464759ce17" +dependencies = [ + "bindgen", + "pkg-config", +] + +[[package]] +name = "libloading" +version = "0.8.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7c4b02199fee7c5d21a5ae7d8cfa79a6ef5bb2fc834d6e9058e89c825efdc55" +dependencies = [ + "cfg-if", + "windows-link", +] + [[package]] name = "libm" version = "0.2.16" @@ -2603,6 +2690,12 @@ version = "0.3.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" +[[package]] +name = "minimal-lexical" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a" + [[package]] name = "miniz_oxide" version = "0.8.9" @@ -2642,6 +2735,16 @@ dependencies = [ "libc", ] +[[package]] +name = "nom" +version = "7.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d273983c5a657a70a3e8f2a01329822f3b8c8172b73826411a55751e404a0a4a" +dependencies = [ + "memchr", + "minimal-lexical", +] + [[package]] name = "nu-ansi-term" version = "0.50.3" @@ -3263,7 +3366,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "27c6023962132f4b30eb4c172c91ce92d933da334c59c23cddee82358ddafb0b" dependencies = [ "anyhow", - "itertools", + "itertools 0.14.0", "proc-macro2", "quote", "syn 2.0.117", @@ -4834,7 +4937,7 @@ dependencies = [ "ferroid", "futures", "http", - "itertools", + "itertools 0.14.0", "log", "memchr", "parse-display", @@ -4911,8 +5014,7 @@ dependencies = [ [[package]] name = "tiberius" version = "0.12.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1446cb4198848d1562301a3340424b4f425ef79f35ef9ee034769a9dd92c10d" +source = "git+https://github.com/prisma/tiberius?rev=a6b4fcdae0de5702427290b89f8d05bc51f3bcfa#a6b4fcdae0de5702427290b89f8d05bc51f3bcfa" dependencies = [ "async-trait", "asynchronous-codec", @@ -4923,6 +5025,7 @@ dependencies = [ "encoding_rs", "enumflags2", "futures-util", + "libgssapi", "num-traits", "once_cell", "pin-project-lite", diff --git a/linux/Cargo.toml b/linux/Cargo.toml index 9235fd13c..c2e3a9519 100644 --- a/linux/Cargo.toml +++ b/linux/Cargo.toml @@ -32,7 +32,9 @@ tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } clickhouse = { version = "0.15", default-features = false, features = ["rustls-tls"] } sqlx = { version = "0.8", default-features = false, features = ["runtime-tokio", "tls-rustls", "chrono", "rust_decimal", "uuid", "json"] } -tiberius = { version = "0.12", default-features = false, features = ["tds73", "rustls", "chrono", "rust_decimal"] } +# `integrated-auth-gssapi` (Windows integrated auth) links MIT Kerberos +# (libkrb5) and runs bindgen (libclang) at build time. +tiberius = { version = "0.12", default-features = false, features = ["tds73", "rustls", "chrono", "rust_decimal", "integrated-auth-gssapi"] } chrono = { version = "0.4", default-features = false, features = ["clock", "serde"] } rust_decimal = { version = "1", default-features = false, features = ["serde", "std"] } futures = "0.3" @@ -51,3 +53,14 @@ relm4 = { version = "0.11", default-features = false, features = ["macros", "lib lto = "fat" codegen-units = 1 strip = "symbols" + +# tiberius 0.12.3 (its latest release) pins libgssapi ^0.4.5. libgssapi 0.4.6's +# `Buf::deref` calls `slice::from_raw_parts(NULL, 0)` on the empty token a +# successful Kerberos handshake returns -- undefined behavior that aborts every +# debug build (`cargo run`). tiberius already fixed this on `main` (merged PR +# prisma/tiberius#372: bump libgssapi 0.4.5 -> 0.8.x, whose deref is guarded) +# but has not cut a release; the bug is tracked open as prisma/tiberius#343. +# Pin the fixed commit until a release lands, then drop this and bump the +# `tiberius` version above to the release. +[patch.crates-io] +tiberius = { git = "https://github.com/prisma/tiberius", rev = "a6b4fcdae0de5702427290b89f8d05bc51f3bcfa" } diff --git a/linux/README.md b/linux/README.md index 476798093..4714ed1ac 100644 --- a/linux/README.md +++ b/linux/README.md @@ -33,13 +33,13 @@ System dependencies: ```bash # Ubuntu / Debian -sudo apt install -y build-essential pkg-config libgtk-4-dev libadwaita-1-dev libgtksourceview-5-dev libssl-dev libsecret-1-dev +sudo apt install -y build-essential pkg-config libgtk-4-dev libadwaita-1-dev libgtksourceview-5-dev libssl-dev libsecret-1-dev libkrb5-dev clang # Fedora -sudo dnf install -y gcc pkg-config gtk4-devel libadwaita-devel gtksourceview5-devel openssl-devel libsecret-devel +sudo dnf install -y gcc pkg-config gtk4-devel libadwaita-devel gtksourceview5-devel openssl-devel libsecret-devel krb5-devel clang # Arch -sudo pacman -S --needed base-devel pkg-config gtk4 libadwaita gtksourceview5 openssl libsecret +sudo pacman -S --needed base-devel pkg-config gtk4 libadwaita gtksourceview5 openssl libsecret krb5 clang ``` Verify the right versions are present: @@ -70,6 +70,39 @@ Driver smoke against a Postgres you already run, no Docker needed: Optional: if the system `-dev` packages above are missing, extract the package payloads under `../.local-deps/root/` (so headers land in `../.local-deps/root/usr/include`) and `source scripts/dev-env.sh` before cargo. Debian-family layouts only. +`libkrb5-dev` and `clang` are there for the SQL Server driver's Windows +integrated auth, which links MIT Kerberos and runs bindgen at build +time. + +## SQL Server with Windows integrated auth + +Pick **Method → Windows (Kerberos)** in the connect dialog. There is no +username or password to enter: the driver uses whatever ticket `klist` +shows, so get one first. + +```bash +kinit you@EXAMPLE.COM +``` + +The driver asks for `MSSQLSvc/:`, built from the host and +port you typed, not from an SSH tunnel's local forward. Three things are +worth knowing: + +- tiberius imports that SPN as a raw Kerberos principal, so it picks up + `default_realm` from `/etc/krb5.conf` and nothing else. `[domain_realm]` + does not apply: that lookup only runs for host-based service names, and + tiberius exposes no SPN override. A server in another realm works only + when your KDC answers with a referral, which Active Directory does + inside a forest. Otherwise the login fails with + `KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN`. +- The host has to match the SPN registered on the server. An IP address + or a CNAME usually does not. +- Running from source is the supported path today. Under Flatpak the + sandbox has no `/etc/krb5.conf` and its `/tmp` is private, so a FILE + ticket cache there is invisible; the manifest grants the config file + and the KCM socket, and a FILE cache needs `KRB5CCNAME` pointed + somewhere under `$HOME`. + ## Documentation index | Topic | File | diff --git a/linux/ROADMAP.md b/linux/ROADMAP.md index b8673a3bc..a232bb065 100644 --- a/linux/ROADMAP.md +++ b/linux/ROADMAP.md @@ -99,6 +99,8 @@ Exit criterion: a developer can demo the basic flows (connect, browse, edit, que - [x] TLS toggle on connect options - [x] SSH tunnelling via `russh` (host, port, key / password auth) - [ ] SSH jump host +- [x] Windows integrated (Kerberos) authentication for SQL Server, from the ambient ticket cache +- [ ] Kerberos against a service outside the client's default realm, which needs an SPN override upstream in tiberius - [x] Read-only mode toggle per connection - [x] Cancel running query: button + Esc shortcut - [ ] `Connection::cancel` driver method, so cancelling stops the server-side query instead of dropping the client future diff --git a/linux/crates/app/src/services/connection_service.rs b/linux/crates/app/src/services/connection_service.rs index 27645a594..3006f62a8 100644 --- a/linux/crates/app/src/services/connection_service.rs +++ b/linux/crates/app/src/services/connection_service.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use secrecy::SecretString; -use tablepro_core::{ConnectOptions, Connection, DriverRegistry, ReadOnlyConnection, TableInfo}; +use tablepro_core::{AuthMode, ConnectOptions, Connection, DriverRegistry, ReadOnlyConnection, TableInfo}; use tablepro_ssh::{SshConfig, SshTunnel}; use tablepro_storage::{SavedConnection, SavedSshAuth, load_password, load_ssh_passphrase, load_ssh_password}; @@ -11,11 +11,16 @@ pub async fn open_saved(registry: Arc, saved: SavedConnection) - let driver = registry .get(&saved.driver_id) .ok_or_else(|| format!("driver {} not registered", saved.driver_id))?; - let password = load_password(saved.id) - .await - .ok() - .flatten() - .unwrap_or_else(|| SecretString::new(String::new().into())); + // Kerberos never had a secret of ours to store, so there is nothing + // to read back. + let password = match saved.auth_mode { + AuthMode::Kerberos => SecretString::new(String::new().into()), + AuthMode::Password => load_password(saved.id) + .await + .ok() + .flatten() + .unwrap_or_else(|| SecretString::new(String::new().into())), + }; let id = saved.id; let ssh_cfg = match &saved.ssh { @@ -30,6 +35,8 @@ pub async fn open_saved(registry: Arc, saved: SavedConnection) - username: saved.username, password, use_tls: saved.use_tls, + auth_mode: saved.auth_mode, + service_endpoint: None, }; let (conn, tunnel) = establish(&*driver, opts.clone(), ssh_cfg.clone(), saved.read_only).await?; @@ -55,23 +62,49 @@ pub async fn establish( ssh: Option, read_only: bool, ) -> Result<(Box, Option), String> { + check_auth_mode(opts.auth_mode, driver.supports_integrated_auth(), driver.display_name())?; let tunnel = if let Some(cfg) = ssh { - let remote_host = std::mem::take(&mut opts.host); - let remote_port = opts.port; - let tun = SshTunnel::open(cfg, remote_host, remote_port) + let remote = (std::mem::take(&mut opts.host), opts.port); + let tun = SshTunnel::open(cfg, remote.0.clone(), remote.1) .await .map_err(|e| format!("ssh: {e}"))?; - opts.host = tun.local_host().to_string(); - opts.port = tun.local_port(); + redirect_through_tunnel(&mut opts, remote, (tun.local_host().to_string(), tun.local_port())); Some(tun) } else { None }; - let raw = driver.connect(opts).await.map_err(|e| format!("connect: {e}"))?; + let raw = driver + .connect(opts) + .await + .map_err(|e| crate::ui::error_text::driver_message(&e))?; let conn = if read_only { ReadOnlyConnection::wrap(raw) } else { raw }; Ok((conn, tunnel)) } +/// The socket has to point at the local forward while the service keeps +/// its own name: without the remembered endpoint Kerberos would ask the +/// KDC for MSSQLSvc/127.0.0.1:, and TLS would validate +/// the certificate against the same wrong name. +fn redirect_through_tunnel(opts: &mut ConnectOptions, remote: (String, u16), local: (String, u16)) { + opts.service_endpoint = Some(remote); + opts.host = local.0; + opts.port = local.1; +} + +/// A saved connection carries its auth mode, so a file edited by hand +/// can name a mode the driver never implements. Password would then be +/// sent as an empty string and the login would fail as a credential +/// problem rather than a configuration one. +fn check_auth_mode(mode: AuthMode, supports_integrated: bool, driver_name: &str) -> Result<(), String> { + if mode == AuthMode::Kerberos && !supports_integrated { + return Err( + crate::tr!("The {driver} driver does not support Windows (Kerberos) authentication.") + .replace("{driver}", driver_name), + ); + } + Ok(()) +} + async fn resolve_saved_ssh(id: uuid::Uuid, saved: &tablepro_storage::SavedSshConfig) -> Result { let auth = match &saved.auth { SavedSshAuth::Password => { @@ -102,3 +135,32 @@ async fn resolve_saved_ssh(id: uuid::Uuid, saved: &tablepro_storage::SavedSshCon auth, }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_tunnel_moves_the_socket_and_keeps_the_service_name() { + let mut opts = ConnectOptions { + host: "127.0.0.1".into(), + port: 54321, + ..Default::default() + }; + redirect_through_tunnel( + &mut opts, + ("sql.corp.example".into(), 1433), + ("127.0.0.1".into(), 54321), + ); + assert_eq!(opts.host, "127.0.0.1"); + assert_eq!(opts.port, 54321); + assert_eq!(opts.service_address(), ("sql.corp.example", 1433)); + } + + #[test] + fn kerberos_is_refused_for_a_driver_that_cannot_perform_it() { + assert!(check_auth_mode(AuthMode::Kerberos, false, "PostgreSQL").is_err()); + assert!(check_auth_mode(AuthMode::Kerberos, true, "SQL Server").is_ok()); + assert!(check_auth_mode(AuthMode::Password, false, "PostgreSQL").is_ok()); + } +} diff --git a/linux/crates/app/src/ui/connect_dialog.rs b/linux/crates/app/src/ui/connect_dialog.rs index c48f29f0f..264a8f355 100644 --- a/linux/crates/app/src/ui/connect_dialog.rs +++ b/linux/crates/app/src/ui/connect_dialog.rs @@ -6,7 +6,7 @@ use relm4::{adw, gtk}; use secrecy::{ExposeSecret, SecretString}; use uuid::Uuid; -use tablepro_core::{ConnectOptions, DriverRegistry, TableInfo}; +use tablepro_core::{AuthMode, ConnectOptions, DriverRegistry, TableInfo}; use tablepro_storage::{ SavedConnection, SavedSshConfig, save_connections, store_password, store_ssh_passphrase, store_ssh_password, }; @@ -24,6 +24,7 @@ pub struct ConnectDialog { database: adw::EntryRow, username: adw::EntryRow, password: adw::PasswordEntryRow, + auth_combo: adw::ComboRow, use_tls: adw::SwitchRow, read_only: adw::SwitchRow, auth_group: adw::PreferencesGroup, @@ -31,6 +32,7 @@ pub struct ConnectDialog { test_button: gtk::Button, submit: gtk::Button, toast_overlay: adw::ToastOverlay, + form: AuthFormState, } #[derive(Debug, Clone)] @@ -39,6 +41,51 @@ struct DriverEntry { display_name: String, } +/// The auth-method model's rows, in the order the combo shows them. +/// The label list and the selection decoder are both derived from this, +/// so a row index can never mean two different things. +const AUTH_MODE_ROWS: [AuthMode; 2] = [AuthMode::Password, AuthMode::Kerberos]; + +fn auth_mode_label(mode: AuthMode) -> String { + match mode { + AuthMode::Password => crate::tr!("Password"), + AuthMode::Kerberos => crate::tr!("Windows (Kerberos)"), + } +} + +fn auth_mode_for_row(row: u32) -> AuthMode { + AUTH_MODE_ROWS.get(row as usize).copied().unwrap_or_default() +} + +/// What the selected driver allows, kept beside the widgets so the form +/// never reads its own visibility flags back to work out the mode. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +struct AuthFormState { + file_based: bool, + supports_integrated: bool, + selected: AuthMode, +} + +impl AuthFormState { + /// A selection left over from another driver resolves back to + /// password auth instead of leaking across the switch. + fn mode(self) -> AuthMode { + if self.shows_method() { + self.selected + } else { + AuthMode::Password + } + } + + fn shows_method(self) -> bool { + !self.file_based && self.supports_integrated + } + + fn shows_credentials(self) -> bool { + !self.file_based && self.mode() == AuthMode::Password + } +} + pub struct ConnectDialogInit { pub registry: Arc, } @@ -48,6 +95,7 @@ pub enum ConnectDialogInput { DriverChanged(u32), SshToggled, SshAuthChanged, + AuthModeChanged, Submit, TestConnection, InputChanged, @@ -187,9 +235,22 @@ impl Component for ConnectDialog { connection_group.add(&port); connection_group.add(&database); + let auth_labels: Vec = AUTH_MODE_ROWS.iter().map(|mode| auth_mode_label(*mode)).collect(); + let auth_labels_ref: Vec<&str> = auth_labels.iter().map(String::as_str).collect(); + let auth_mode_model = gtk::StringList::new(&auth_labels_ref); + let auth_combo = adw::ComboRow::builder() + .title(crate::tr!("Method")) + .model(&auth_mode_model) + .build(); + let sender_for_authmode = sender.clone(); + auth_combo.connect_selected_notify(move |_| { + sender_for_authmode.input(ConnectDialogInput::AuthModeChanged); + }); + let auth_group = adw::PreferencesGroup::builder() .title(crate::tr!("Authentication")) .build(); + auth_group.add(&auth_combo); auth_group.add(&username); auth_group.add(&password); @@ -218,7 +279,7 @@ impl Component for ConnectDialog { let toast_overlay = adw::ToastOverlay::new(); toast_overlay.set_child(Some(&page)); - let model = ConnectDialog { + let mut model = ConnectDialog { registry: init.registry, drivers: drivers.clone(), driver_combo, @@ -227,6 +288,7 @@ impl Component for ConnectDialog { database, username, password, + auth_combo, use_tls, read_only, auth_group, @@ -234,6 +296,7 @@ impl Component for ConnectDialog { test_button, submit, toast_overlay, + form: AuthFormState::default(), }; let widgets = view_output!(); @@ -258,7 +321,7 @@ impl Component for ConnectDialog { fn update(&mut self, msg: Self::Input, sender: ComponentSender, root: &Self::Root) { match msg { ConnectDialogInput::DriverChanged(idx) => { - let Some(entry) = self.drivers.get(idx as usize) else { + let Some(entry) = self.drivers.get(idx as usize).cloned() else { return; }; if let Some(driver) = self.registry.get(&entry.id) { @@ -266,6 +329,7 @@ impl Component for ConnectDialog { self.port.set_value(driver.default_port() as f64); } root.set_title(&crate::tr!("Connect to {name}").replace("{name}", &entry.display_name)); + self.refresh_validity(); } ConnectDialogInput::SshToggled => { @@ -277,6 +341,12 @@ impl Component for ConnectDialog { self.refresh_validity(); } + ConnectDialogInput::AuthModeChanged => { + self.form.selected = auth_mode_for_row(self.auth_combo.selected()); + self.apply_form_state(); + self.refresh_validity(); + } + ConnectDialogInput::InputChanged => { self.refresh_validity(); } @@ -300,17 +370,12 @@ impl Component for ConnectDialog { } }; - let opts = ConnectOptions { - host: self.host.text().to_string(), - port: self.port.value() as u16, - database: self.database.text().to_string(), - username: self.username.text().to_string(), - password: SecretString::new(self.password.text().to_string().into()), - use_tls: self.use_tls.is_active(), - }; + let opts = self.collect_options(); let label = if entry.id == "sqlite" { opts.database.clone() + } else if opts.auth_mode == AuthMode::Kerberos { + opts.host.clone() } else { format!("{}@{}", opts.username, opts.host) }; @@ -355,14 +420,7 @@ impl Component for ConnectDialog { self.show_toast(&crate::tr!("Driver {id} not registered").replace("{id}", &entry.id)); return; }; - let opts = ConnectOptions { - host: self.host.text().to_string(), - port: self.port.value() as u16, - database: self.database.text().to_string(), - username: self.username.text().to_string(), - password: SecretString::new(self.password.text().to_string().into()), - use_tls: self.use_tls.is_active(), - }; + let opts = self.collect_options(); let ssh_inputs = if self.ssh.is_enabled() { match self.ssh.collect() { Ok(inputs) => Some(inputs.cfg), @@ -431,11 +489,11 @@ impl ConnectDialog { let database_empty = self.database.text().trim().is_empty(); toggle_error(&self.database, database_empty); - let host_required = self.host.is_visible(); + let host_required = !self.form.file_based; let host_empty = host_required && self.host.text().trim().is_empty(); toggle_error(&self.host, host_empty); - let username_required = self.username.is_visible(); + let username_required = self.form.shows_credentials(); let username_empty = username_required && self.username.text().trim().is_empty(); toggle_error(&self.username, username_empty); @@ -448,11 +506,11 @@ impl ConnectDialog { if self.database.text().trim().is_empty() { return false; } - if self.host.is_visible() { + if !self.form.file_based { if self.host.text().trim().is_empty() { return false; } - if self.username.text().trim().is_empty() { + if self.form.shows_credentials() && self.username.text().trim().is_empty() { return false; } } @@ -462,24 +520,53 @@ impl ConnectDialog { true } - fn apply_driver_form_visibility(&self, driver: &dyn tablepro_core::DatabaseDriver) { - let file_based = driver.is_file_based(); - self.host.set_visible(!file_based); - self.port.set_visible(!file_based); - self.username.set_visible(!file_based); - self.password.set_visible(!file_based); - self.use_tls.set_visible(!file_based); - // For file-based drivers (SQLite), only Connection + Options - // groups make sense; hide Authentication and SSH entirely. - self.auth_group.set_visible(!file_based); - self.ssh.set_visible(!file_based); - self.database.set_title(&if file_based { + fn apply_driver_form_visibility(&mut self, driver: &dyn tablepro_core::DatabaseDriver) { + self.form.file_based = driver.is_file_based(); + self.form.supports_integrated = driver.supports_integrated_auth(); + self.apply_form_state(); + self.database.set_title(&if self.form.file_based { crate::tr!("File path") } else { crate::tr!("Database") }); } + fn apply_form_state(&self) { + let network = !self.form.file_based; + self.host.set_visible(network); + self.port.set_visible(network); + self.use_tls.set_visible(network); + // For file-based drivers (SQLite), only Connection + Options + // groups make sense; hide Authentication and SSH entirely. + self.auth_group.set_visible(network); + self.ssh.set_visible(network); + self.auth_combo.set_visible(self.form.shows_method()); + let credentials = self.form.shows_credentials(); + self.username.set_visible(credentials); + self.password.set_visible(credentials); + } + + fn collect_options(&self) -> ConnectOptions { + // The credential rows keep their text while hidden, so a mode or + // driver that does not use them must drop it here rather than + // let it reach the driver and the keyring. + let (username, password) = if self.form.shows_credentials() { + (self.username.text().to_string(), self.password.text().to_string()) + } else { + (String::new(), String::new()) + }; + ConnectOptions { + host: self.host.text().to_string(), + port: self.port.value() as u16, + database: self.database.text().to_string(), + username, + password: SecretString::new(password.into()), + use_tls: self.use_tls.is_active(), + auth_mode: self.form.mode(), + service_endpoint: None, + } + } + fn show_toast(&self, message: &str) { self.toast_overlay.add_toast(adw::Toast::new(message)); } @@ -536,7 +623,7 @@ async fn run_connect( connection_service::establish(driver.as_ref(), opts.clone(), ssh_for_establish, read_only).await?; let tables = conn.list_tables().await.map_err(|e| format!("list_tables: {e}"))?; - let id = match find_existing_id(&driver_id, &opts_clone, ssh.as_ref()).await { + let id = match find_existing_id(&driver_id, &opts_clone, driver.is_file_based(), ssh.as_ref()).await { Some(id) => id, None => Uuid::new_v4(), }; @@ -551,6 +638,7 @@ async fn run_connect( username: opts_clone.username.clone(), use_tls: opts_clone.use_tls, read_only, + auth_mode: opts_clone.auth_mode, ssh: ssh.as_ref().map(|s| s.saved.clone()), // Stays None until `App::on_connected` stamps it. Save then // connect arrives in that order, so a freshly-saved entry is @@ -559,7 +647,9 @@ async fn run_connect( }; save_one(&saved).await.map_err(|e| format!("save: {e}"))?; - let _ = store_password(saved.id, stored_password.expose_secret(), &label).await; + if saved.auth_mode == AuthMode::Password { + let _ = store_password(saved.id, stored_password.expose_secret(), &label).await; + } if let Some(s) = &ssh { match &s.secret_to_store { SshSecretToStore::Password(p) => { @@ -594,21 +684,42 @@ async fn save_one(connection: &SavedConnection) -> Result<(), tablepro_storage:: save_connections(&existing).await } -async fn find_existing_id(driver_id: &str, opts: &ConnectOptions, ssh: Option<&SshInputs>) -> Option { +async fn find_existing_id( + driver_id: &str, + opts: &ConnectOptions, + file_based: bool, + ssh: Option<&SshInputs>, +) -> Option { let existing = tablepro_storage::load_connections().await.ok()?; existing .into_iter() - .find(|c| { - c.driver_id == driver_id - && c.host == opts.host - && c.port == opts.port - && c.database == opts.database - && c.username == opts.username - && saved_ssh_matches(&c.ssh, ssh) - }) + .find(|c| matches_existing(c, driver_id, opts, file_based, ssh)) .map(|c| c.id) } +fn matches_existing( + saved: &SavedConnection, + driver_id: &str, + opts: &ConnectOptions, + file_based: bool, + ssh: Option<&SshInputs>, +) -> bool { + if saved.driver_id != driver_id || saved.database != opts.database { + return false; + } + // A file-based driver is reached by its path alone. Comparing the + // credentials there would strand every entry an older build wrote + // with the hidden Username row's leftover text. + if file_based { + return true; + } + saved.host == opts.host + && saved.port == opts.port + && saved.username == opts.username + && saved.auth_mode == opts.auth_mode + && saved_ssh_matches(&saved.ssh, ssh) +} + fn saved_ssh_matches(saved: &Option, current: Option<&SshInputs>) -> bool { match (saved, current) { (None, None) => true, @@ -616,3 +727,153 @@ fn saved_ssh_matches(saved: &Option, current: Option<&SshInputs> _ => false, } } + +#[cfg(test)] +mod tests { + use super::*; + + /// (state, mode, shows_method, shows_credentials) + #[test] + fn auth_form_state_drives_mode_and_visibility() { + let cases = [ + (AuthFormState::default(), AuthMode::Password, false, true), + // MSSQL: offers the selector, password until Kerberos is picked. + ( + AuthFormState { + file_based: false, + supports_integrated: true, + selected: AuthMode::Password, + }, + AuthMode::Password, + true, + true, + ), + ( + AuthFormState { + file_based: false, + supports_integrated: true, + selected: AuthMode::Kerberos, + }, + AuthMode::Kerberos, + true, + false, + ), + // Postgres: a stale Kerberos selection does not survive the switch. + ( + AuthFormState { + file_based: false, + supports_integrated: false, + selected: AuthMode::Kerberos, + }, + AuthMode::Password, + false, + true, + ), + // SQLite: no credentials at all. + ( + AuthFormState { + file_based: true, + supports_integrated: true, + selected: AuthMode::Kerberos, + }, + AuthMode::Password, + false, + false, + ), + ]; + for (state, mode, method, credentials) in cases { + assert_eq!(state.mode(), mode, "{state:?}"); + assert_eq!(state.shows_method(), method, "{state:?}"); + assert_eq!(state.shows_credentials(), credentials, "{state:?}"); + } + } + + #[test] + fn the_combo_rows_decode_to_the_modes_they_are_labelled_with() { + assert_eq!(auth_mode_for_row(0), AuthMode::Password); + assert_eq!(auth_mode_for_row(1), AuthMode::Kerberos); + assert_eq!(auth_mode_for_row(7), AuthMode::Password); + assert_eq!(AUTH_MODE_ROWS.len(), 2); + } + + fn saved(driver_id: &str, username: &str, auth_mode: AuthMode) -> SavedConnection { + SavedConnection { + id: Uuid::new_v4(), + name: "saved".into(), + driver_id: driver_id.into(), + host: "sql.corp.example".into(), + port: 1433, + database: "sales".into(), + username: username.into(), + use_tls: false, + read_only: false, + auth_mode, + ssh: None, + last_opened_at: None, + } + } + + fn opts(username: &str, auth_mode: AuthMode) -> ConnectOptions { + ConnectOptions { + host: "sql.corp.example".into(), + port: 1433, + database: "sales".into(), + username: username.into(), + auth_mode, + ..Default::default() + } + } + + #[test] + fn a_file_based_entry_is_identified_by_its_path_alone() { + let legacy = saved("sqlite", "postgres", AuthMode::Password); + assert!(matches_existing( + &legacy, + "sqlite", + &opts("", AuthMode::Password), + true, + None + )); + } + + #[test] + fn a_network_entry_still_distinguishes_user_and_auth_mode() { + let entry = saved("mssql", "sa", AuthMode::Password); + assert!(matches_existing( + &entry, + "mssql", + &opts("sa", AuthMode::Password), + false, + None + )); + assert!(!matches_existing( + &entry, + "mssql", + &opts("other", AuthMode::Password), + false, + None + )); + assert!(!matches_existing( + &entry, + "mssql", + &opts("", AuthMode::Kerberos), + false, + None + )); + } + + #[test] + fn two_kerberos_entries_on_one_host_are_told_apart_by_database() { + let sales = saved("mssql", "", AuthMode::Kerberos); + let mut finance = opts("", AuthMode::Kerberos); + finance.database = "finance".into(); + assert!(matches_existing( + &sales, + "mssql", + &opts("", AuthMode::Kerberos), + false, + None + )); + assert!(!matches_existing(&sales, "mssql", &finance, false, None)); + } +} diff --git a/linux/crates/app/src/ui/connection_row.rs b/linux/crates/app/src/ui/connection_row.rs index 859781121..749a88b47 100644 --- a/linux/crates/app/src/ui/connection_row.rs +++ b/linux/crates/app/src/ui/connection_row.rs @@ -3,6 +3,7 @@ use relm4::factory::{DynamicIndex, FactoryComponent, FactorySender}; use relm4::{adw, gtk}; use uuid::Uuid; +use tablepro_core::AuthMode; use tablepro_storage::SavedConnection; #[derive(Debug)] @@ -114,8 +115,44 @@ impl FactoryComponent for ConnectionRow { fn subtitle_for(saved: &SavedConnection) -> String { if saved.driver_id == "sqlite" { - format!("sqlite · {}", saved.database) - } else { - format!("{} · {}@{}:{}", saved.driver_id, saved.username, saved.host, saved.port) + return format!("sqlite · {}", saved.database); + } + match saved.auth_mode { + AuthMode::Kerberos => format!("{} · {}:{}", saved.driver_id, saved.host, saved.port), + AuthMode::Password => format!("{} · {}@{}:{}", saved.driver_id, saved.username, saved.host, saved.port), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn saved(username: &str, auth_mode: AuthMode) -> SavedConnection { + SavedConnection { + id: Uuid::new_v4(), + name: "Corp".into(), + driver_id: "mssql".into(), + host: "sql.corp.example".into(), + port: 1433, + database: "sales".into(), + username: username.into(), + use_tls: true, + read_only: false, + auth_mode, + ssh: None, + last_opened_at: None, + } + } + + #[test] + fn a_kerberos_row_has_no_username_separator_to_dangle() { + assert_eq!( + subtitle_for(&saved("", AuthMode::Kerberos)), + "mssql · sql.corp.example:1433" + ); + assert_eq!( + subtitle_for(&saved("sa", AuthMode::Password)), + "mssql · sa@sql.corp.example:1433" + ); } } diff --git a/linux/crates/app/src/ui/error_text.rs b/linux/crates/app/src/ui/error_text.rs index 7d43e9d73..ca1909c30 100644 --- a/linux/crates/app/src/ui/error_text.rs +++ b/linux/crates/app/src/ui/error_text.rs @@ -32,6 +32,10 @@ pub fn driver_message(error: &DriverError) -> String { crate::tr!("This connection is read-only. Reopen it without read-only mode to make changes.") } DriverError::Internal(detail) => crate::tr!("Internal driver error: {detail}").replace("{detail}", detail), + DriverError::IntegratedAuth(detail) => crate::tr!( + "Kerberos login failed: {detail}. Check that klist shows a valid ticket, run kinit if it does not, and make sure the server's SPN matches the host you typed." + ) + .replace("{detail}", detail), DriverError::Transaction { statement_index, source, @@ -79,4 +83,12 @@ mod tests { assert!(driver_message(&DriverError::AuthFailed).contains("wrong")); assert!(driver_message(&DriverError::Disconnected).contains("Try reconnecting")); } + + #[test] + fn integrated_auth_names_the_remedy_and_keeps_the_gssapi_detail() { + let message = driver_message(&DriverError::IntegratedAuth("No Kerberos credentials available".into())); + assert!(message.contains("No Kerberos credentials available")); + assert!(message.contains("kinit")); + assert!(message.contains("SPN")); + } } diff --git a/linux/crates/app/src/ui/mod.rs b/linux/crates/app/src/ui/mod.rs index 61681274d..b73300522 100644 --- a/linux/crates/app/src/ui/mod.rs +++ b/linux/crates/app/src/ui/mod.rs @@ -4,7 +4,7 @@ mod cell_editor; mod connect_dialog; mod connection_row; mod editor; -mod error_text; +pub(crate) mod error_text; mod filter_strip; mod grid; mod history_dialog; diff --git a/linux/crates/core/src/connection.rs b/linux/crates/core/src/connection.rs index 6cf450e9e..843da4400 100644 --- a/linux/crates/core/src/connection.rs +++ b/linux/crates/core/src/connection.rs @@ -1,9 +1,24 @@ use async_trait::async_trait; use secrecy::SecretString; +use serde::{Deserialize, Serialize}; use crate::error::DriverError; use crate::query::{ColumnInfo, ExecResult, ForeignKeyInfo, IndexInfo, QueryResult, TableInfo, Value}; +/// How a driver authenticates to the database. Most drivers only +/// support [`AuthMode::Password`]; the SQL Server driver also supports +/// [`AuthMode::Kerberos`] (Windows integrated auth) using the current +/// user's Kerberos ticket cache obtained via `kinit`. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum AuthMode { + #[default] + Password, + /// Windows integrated authentication over Kerberos (GSSAPI), using + /// the ambient ticket cache. `username`/`password` are ignored. + Kerberos, +} + #[derive(Debug, Clone)] pub struct ConnectOptions { pub host: String, @@ -12,6 +27,20 @@ pub struct ConnectOptions { pub username: String, pub password: SecretString, pub use_tls: bool, + pub auth_mode: AuthMode, + /// Set only when `host`/`port` were replaced by a tunnel's local + /// forward. `None` means the socket already points at the service. + pub service_endpoint: Option<(String, u16)>, +} + +impl ConnectOptions { + /// Host and port the service answers to, which is `host`/`port` + /// unless a tunnel replaced them. + pub fn service_address(&self) -> (&str, u16) { + self.service_endpoint + .as_ref() + .map_or((self.host.as_str(), self.port), |(host, port)| (host.as_str(), *port)) + } } impl Default for ConnectOptions { @@ -23,6 +52,8 @@ impl Default for ConnectOptions { username: String::new(), password: SecretString::new(String::new().into()), use_tls: false, + auth_mode: AuthMode::Password, + service_endpoint: None, } } } @@ -82,3 +113,26 @@ pub trait Connection: Send + Sync { async fn ping(&self) -> Result<(), DriverError>; async fn close(self: Box) -> Result<(), DriverError>; } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn service_address_prefers_the_tunnelled_service_over_the_socket() { + let direct = ConnectOptions { + host: "sql.corp.example".into(), + port: 1433, + ..Default::default() + }; + assert_eq!(direct.service_address(), ("sql.corp.example", 1433)); + + let tunnelled = ConnectOptions { + host: "127.0.0.1".into(), + port: 54321, + service_endpoint: Some(("sql.corp.example".into(), 1433)), + ..Default::default() + }; + assert_eq!(tunnelled.service_address(), ("sql.corp.example", 1433)); + } +} diff --git a/linux/crates/core/src/driver.rs b/linux/crates/core/src/driver.rs index c6921857e..0847741f5 100644 --- a/linux/crates/core/src/driver.rs +++ b/linux/crates/core/src/driver.rs @@ -34,5 +34,16 @@ pub trait DatabaseDriver: Send + Sync { true } + /// Whether this driver supports Windows integrated / Kerberos + /// authentication (`ConnectOptions::auth_mode == AuthMode::Kerberos`). + /// The connect dialog shows the auth-mode selector only for drivers + /// returning `true`, and while Kerberos is selected it hides the + /// username and password rows and sends empty credentials. Say + /// `true` only if `connect` maps `AuthMode::Kerberos` onto a real + /// integrated-auth path; `establish` refuses the mode otherwise. + fn supports_integrated_auth(&self) -> bool { + false + } + async fn connect(&self, opts: ConnectOptions) -> Result, DriverError>; } diff --git a/linux/crates/core/src/error.rs b/linux/crates/core/src/error.rs index 771adf4cb..4eb751e02 100644 --- a/linux/crates/core/src/error.rs +++ b/linux/crates/core/src/error.rs @@ -33,4 +33,11 @@ pub enum DriverError { statement_index: usize, source: Box, }, + + /// Integrated (Kerberos / GSSAPI) authentication could not complete. + /// The payload is the GSSAPI major and minor status text, which is + /// what distinguishes a missing ticket from an expired one, an + /// unknown SPN, or an unreachable KDC. + #[error("integrated authentication failed: {0}")] + IntegratedAuth(String), } diff --git a/linux/crates/core/src/lib.rs b/linux/crates/core/src/lib.rs index e728b4577..c36bdd842 100644 --- a/linux/crates/core/src/lib.rs +++ b/linux/crates/core/src/lib.rs @@ -8,7 +8,7 @@ mod registry; pub mod sql_ddl; pub mod sql_dialect; -pub use connection::{ConnectOptions, Connection}; +pub use connection::{AuthMode, ConnectOptions, Connection}; pub use driver::DatabaseDriver; pub use error::DriverError; pub use filter::{BuildFilterError, Combinator, FilterOp, FilterRule, FilterSet, FilterValue, build_filter_where}; diff --git a/linux/crates/drivers/clickhouse/tests/integration.rs b/linux/crates/drivers/clickhouse/tests/integration.rs index fb3870fba..20b996d5c 100644 --- a/linux/crates/drivers/clickhouse/tests/integration.rs +++ b/linux/crates/drivers/clickhouse/tests/integration.rs @@ -30,6 +30,7 @@ async fn start_clickhouse() -> (ContainerAsync, ConnectOptions) { username: "default".into(), password: secrecy::SecretString::new("tablepro".to_string().into()), use_tls: false, + ..Default::default() }; (container, opts) } diff --git a/linux/crates/drivers/mssql/src/lib.rs b/linux/crates/drivers/mssql/src/lib.rs index a518d5a9a..9b64b6f79 100644 --- a/linux/crates/drivers/mssql/src/lib.rs +++ b/linux/crates/drivers/mssql/src/lib.rs @@ -12,8 +12,8 @@ use tokio_util::compat::{Compat, TokioAsyncWriteCompatExt}; use tablepro_core::sql_dialect::build_order_and_pagination; use tablepro_core::{ - ColumnInfo, ConnectOptions, Connection, DatabaseDriver, DriverError, ExecResult, ForeignKeyInfo, IndexInfo, - MAX_QUERY_ROWS, QueryResult, TableInfo, Value, + AuthMode, ColumnInfo, ConnectOptions, Connection, DatabaseDriver, DriverError, ExecResult, ForeignKeyInfo, + IndexInfo, MAX_QUERY_ROWS, QueryResult, TableInfo, Value, }; type MssqlClient = Client>; @@ -42,46 +42,102 @@ impl DatabaseDriver for MssqlDriver { true } + fn supports_integrated_auth(&self) -> bool { + true + } + async fn connect(&self, opts: ConnectOptions) -> Result, DriverError> { - let mut config = Config::new(); - config.host(&opts.host); - config.port(opts.port); - config.database(&opts.database); - config.authentication(AuthMethod::sql_server(&opts.username, opts.password.expose_secret())); - // SQL Server always encrypts the login exchange; `Off` keeps the - // post-login stream in the clear, `Required` encrypts everything. - // No cert-path UI exists, so the server certificate is trusted - // without verification, matching the sqlx drivers' Require / - // Required modes. - config.encryption(if opts.use_tls { - EncryptionLevel::Required - } else { - EncryptionLevel::Off - }); - config.trust_cert(); + let target = build_target(&opts); + + // Integrated auth drives MIT Kerberos through synchronous FFI + // inside tiberius' login, so that future blocks its thread + // between polls instead of yielding. The runtime the app drives + // this from has a single worker: blocking it stalls every other + // task and leaves `timeout` with no thread to fire on. The + // blocking pool is where a blocking poll belongs, and it keeps + // the deadline below enforceable. An attempt that loses the + // race keeps running there and drops its own client. + let handle = tokio::runtime::Handle::current(); + let connecting = tokio::task::spawn_blocking(move || handle.block_on(open_client(target))); // Neither the TCP dial nor the TDS login has its own deadline, // and an unreachable host would otherwise hang the connect // dialog for the OS SYN timeout. The budget covers both so the // failure arrives on the same scale as the sqlx drivers' // acquire_timeout. - tokio::time::timeout(CONNECT_TIMEOUT, async { - let tcp = TcpStream::connect(config.get_addr()).await.map_err(map_io_error)?; - tcp.set_nodelay(true).map_err(map_io_error)?; - Client::connect(config, tcp.compat_write()) - .await - .map_err(map_tiberius_error) - }) - .await - .map_err(|_| DriverError::ConnectionRefused)? - .map(|client| { - Box::new(MssqlConnection { - client: Mutex::new(client), - }) as Box - }) + let client = match tokio::time::timeout(CONNECT_TIMEOUT, connecting).await { + Ok(Ok(opened)) => opened?, + Ok(Err(join)) => return Err(DriverError::Internal(join.to_string())), + Err(_) => return Err(DriverError::ConnectionRefused), + }; + + Ok(Box::new(MssqlConnection { + client: Mutex::new(client), + })) } } +/// Where the client talks and who it says it is talking to. tiberius +/// derives the Kerberos SPN and the TLS server name from the configured +/// host and port, while the socket is opened separately. An SSH tunnel +/// replaces `opts.host`/`opts.port` with a local forward, so the two +/// come from different places: `service_address()` names the server, +/// `dial_host`/`dial_port` reach it. +struct MssqlTarget { + config: Config, + dial_host: String, + dial_port: u16, +} + +fn build_target(opts: &ConnectOptions) -> MssqlTarget { + let (service_host, service_port) = opts.service_address(); + let mut config = Config::new(); + config.host(service_host); + config.port(service_port); + config.database(&opts.database); + config.authentication(auth_method(opts)); + // SQL Server always encrypts the login exchange; `Off` keeps the + // post-login stream in the clear, `Required` encrypts everything. + // No cert-path UI exists, so the server certificate is trusted + // without verification, matching the sqlx drivers' Require / + // Required modes. + config.encryption(if opts.use_tls { + EncryptionLevel::Required + } else { + EncryptionLevel::Off + }); + config.trust_cert(); + MssqlTarget { + config, + dial_host: dial_host(&opts.host).to_string(), + dial_port: opts.port, + } +} + +fn auth_method(opts: &ConnectOptions) -> AuthMethod { + match opts.auth_mode { + AuthMode::Password => AuthMethod::sql_server(&opts.username, opts.password.expose_secret()), + AuthMode::Kerberos => AuthMethod::Integrated, + } +} + +/// `.` is SQL Server shorthand for the local machine. tiberius resolves +/// it on the config side; the socket has to be given the same treatment +/// or the shorthand reaches the resolver verbatim. +fn dial_host(host: &str) -> &str { + if host == "." { "localhost" } else { host } +} + +async fn open_client(target: MssqlTarget) -> Result { + let tcp = TcpStream::connect((target.dial_host.as_str(), target.dial_port)) + .await + .map_err(map_io_error)?; + tcp.set_nodelay(true).map_err(map_io_error)?; + Client::connect(target.config, tcp.compat_write()) + .await + .map_err(map_tiberius_error) +} + struct MssqlConnection { client: Mutex, } @@ -665,6 +721,7 @@ fn map_tiberius_error(err: tiberius::error::Error) -> DriverError { } } } + E::Gssapi(detail) => DriverError::IntegratedAuth(detail), E::Routing { host, port } => DriverError::Internal(format!("server requested routing to {host}:{port}")), other => DriverError::Internal(other.to_string()), } @@ -681,6 +738,71 @@ mod tests { assert_eq!(d.display_name(), "SQL Server"); assert_eq!(d.default_port(), 1433); assert!(!d.is_file_based()); + assert!(d.supports_integrated_auth()); + } + + fn direct_opts() -> ConnectOptions { + ConnectOptions { + host: "sql.corp.example".into(), + port: 1433, + database: "sales".into(), + ..Default::default() + } + } + + #[test] + fn a_direct_connection_names_and_dials_the_same_endpoint() { + let target = build_target(&direct_opts()); + assert_eq!(target.config.get_addr(), "sql.corp.example:1433"); + assert_eq!( + (target.dial_host.as_str(), target.dial_port), + ("sql.corp.example", 1433) + ); + } + + #[test] + fn a_tunnelled_connection_names_the_service_but_dials_the_forward() { + let opts = ConnectOptions { + host: "127.0.0.1".into(), + port: 54321, + service_endpoint: Some(("sql.corp.example".into(), 1433)), + ..direct_opts() + }; + let target = build_target(&opts); + // The SPN and the TLS server name follow this, so a tunnel must + // not push 127.0.0.1 into it. + assert_eq!(target.config.get_addr(), "sql.corp.example:1433"); + assert_eq!((target.dial_host.as_str(), target.dial_port), ("127.0.0.1", 54321)); + } + + #[test] + fn the_local_shorthand_reaches_the_socket_as_localhost() { + let opts = ConnectOptions { + host: ".".into(), + ..direct_opts() + }; + let target = build_target(&opts); + assert_eq!(target.config.get_addr(), "localhost:1433"); + assert_eq!(target.dial_host, "localhost"); + } + + #[test] + fn kerberos_authenticates_from_the_ticket_cache_and_ignores_credentials() { + let opts = ConnectOptions { + auth_mode: AuthMode::Kerberos, + username: "leftover".into(), + ..direct_opts() + }; + assert_eq!(auth_method(&opts), AuthMethod::Integrated); + assert_eq!(auth_method(&direct_opts()), AuthMethod::sql_server("", "")); + } + + #[test] + fn a_gssapi_failure_is_classified_instead_of_reported_as_an_internal_error() { + let err = map_tiberius_error(tiberius::error::Error::Gssapi( + "No Kerberos credentials available".into(), + )); + assert!(matches!(err, DriverError::IntegratedAuth(detail) if detail.contains("No Kerberos"))); } #[test] diff --git a/linux/crates/drivers/mssql/tests/integration.rs b/linux/crates/drivers/mssql/tests/integration.rs index a919410aa..f505b25f0 100644 --- a/linux/crates/drivers/mssql/tests/integration.rs +++ b/linux/crates/drivers/mssql/tests/integration.rs @@ -25,6 +25,7 @@ async fn start_mssql() -> (ContainerAsync, ConnectOptions) { username: "sa".into(), password: SecretString::new(MssqlServer::DEFAULT_SA_PASSWORD.to_string().into()), use_tls: false, + ..Default::default() }; (container, opts) } diff --git a/linux/crates/drivers/mysql/tests/integration.rs b/linux/crates/drivers/mysql/tests/integration.rs index ac6cb3e1f..e19486b46 100644 --- a/linux/crates/drivers/mysql/tests/integration.rs +++ b/linux/crates/drivers/mysql/tests/integration.rs @@ -27,6 +27,7 @@ async fn start_mysql() -> (ContainerAsync, ConnectOptions) { username: "root".into(), password: secrecy::SecretString::new("tablepro_test".to_string().into()), use_tls: false, + ..Default::default() }; (container, opts) } diff --git a/linux/crates/drivers/postgres/tests/integration.rs b/linux/crates/drivers/postgres/tests/integration.rs index a47d08bc5..077bda999 100644 --- a/linux/crates/drivers/postgres/tests/integration.rs +++ b/linux/crates/drivers/postgres/tests/integration.rs @@ -32,6 +32,7 @@ async fn start_pg() -> (ContainerAsync, ConnectOptions) { username: "postgres".into(), password: secrecy::SecretString::new("postgres".to_string().into()), use_tls: false, + ..Default::default() }; (container, opts) } diff --git a/linux/crates/drivers/postgres/tests/smoke_local.rs b/linux/crates/drivers/postgres/tests/smoke_local.rs index 24d174e3c..24b0b222a 100644 --- a/linux/crates/drivers/postgres/tests/smoke_local.rs +++ b/linux/crates/drivers/postgres/tests/smoke_local.rs @@ -32,6 +32,7 @@ fn opts_from_env() -> ConnectOptions { .into(), ), use_tls: false, + ..Default::default() } } diff --git a/linux/crates/storage/src/connections.rs b/linux/crates/storage/src/connections.rs index e2473483b..af9a977c0 100644 --- a/linux/crates/storage/src/connections.rs +++ b/linux/crates/storage/src/connections.rs @@ -2,6 +2,7 @@ use std::path::{Path, PathBuf}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; +use tablepro_core::AuthMode; use uuid::Uuid; use crate::error::StorageError; @@ -20,6 +21,8 @@ pub struct SavedConnection { pub use_tls: bool, #[serde(default)] pub read_only: bool, + #[serde(default)] + pub auth_mode: AuthMode, #[serde(default, skip_serializing_if = "Option::is_none")] pub ssh: Option, /// Last successful open of this connection. Drives the welcome @@ -151,6 +154,7 @@ mod tests { username: "postgres".into(), use_tls: false, read_only: false, + auth_mode: AuthMode::Password, ssh: None, last_opened_at: None, } @@ -228,4 +232,52 @@ mod tests { let loaded = load_from(&path).await.unwrap(); assert_eq!(loaded, vec![conn]); } + + #[tokio::test] + async fn auth_mode_defaults_to_password_on_a_legacy_file() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("connections.json"); + let id = Uuid::new_v4(); + let legacy = format!( + r#"{{"version":1,"connections":[{{ + "id":"{id}","name":"Old","driver_id":"mssql", + "host":"localhost","port":1433,"database":"db", + "username":"sa","use_tls":false}}]}}"# + ); + tokio::fs::write(&path, legacy).await.unwrap(); + let loaded = load_from(&path).await.unwrap(); + assert_eq!(loaded[0].auth_mode, AuthMode::Password); + } + + #[tokio::test] + async fn kerberos_is_written_as_snake_case_and_reads_back() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("connections.json"); + let mut conn = sample_connection(); + conn.auth_mode = AuthMode::Kerberos; + save_to(&path, &[conn.clone()]).await.unwrap(); + let raw: serde_json::Value = serde_json::from_slice(&tokio::fs::read(&path).await.unwrap()).unwrap(); + assert_eq!(raw["connections"][0]["auth_mode"], "kerberos"); + assert_eq!(load_from(&path).await.unwrap(), vec![conn]); + } + + /// Pins the reader against a file already on disk. Renaming the + /// variant fails here instead of orphaning every saved connection: + /// an unparseable file loads as empty, and the next successful + /// connect writes that empty list back. + #[tokio::test] + async fn a_file_written_with_kerberos_still_loads() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("connections.json"); + let id = Uuid::new_v4(); + let on_disk = format!( + r#"{{"version":1,"connections":[{{ + "id":"{id}","name":"Corp","driver_id":"mssql", + "host":"sql.corp.example","port":1433,"database":"sales", + "username":"","use_tls":true,"auth_mode":"kerberos"}}]}}"# + ); + tokio::fs::write(&path, on_disk).await.unwrap(); + let loaded = load_from(&path).await.unwrap(); + assert_eq!(loaded[0].auth_mode, AuthMode::Kerberos); + } } diff --git a/linux/docs/adding-drivers.md b/linux/docs/adding-drivers.md index 088de7bb0..6bb46903a 100644 --- a/linux/docs/adding-drivers.md +++ b/linux/docs/adding-drivers.md @@ -144,6 +144,8 @@ Notes: - `ddl_is_transactional()`: the structure editor batches DDL into one transaction when true. False for engines that commit implicitly on every DDL statement. - `reports_rows_affected()`: the inline-edit Save path reads a zero `rows_affected` on an UPDATE or DELETE as another session having changed the row. Return false if the engine cannot produce a count, or every successful save warns about a lost update. +- `is_file_based()`: the connect dialog hides host, port, TLS, the Authentication group and SSH, and relabels Database to File path. True only for engines that open a local file. +- `supports_integrated_auth()`: the connect dialog shows the Method selector (Password / Windows (Kerberos)) only for drivers returning true, and while Kerberos is selected it hides the username and password rows and sends empty credentials. Return true only if `connect()` maps `AuthMode::Kerberos` onto a real integrated-auth path that reads the ambient Kerberos ticket cache; `connection_service::establish` refuses the mode for every other driver. If your engine needs a different SQL spelling for a statement the app builds centrally, add the dialect branch in `core::sql_dialect` (`quote_ident`, `placeholder_for`, `build_update`, `build_order_and_pagination`) rather than rewriting the SQL inside the driver. ClickHouse takes `build_update`'s `ALTER TABLE … UPDATE` branch for this reason. diff --git a/linux/docs/testing.md b/linux/docs/testing.md index 2863899a0..259af6630 100644 --- a/linux/docs/testing.md +++ b/linux/docs/testing.md @@ -35,7 +35,7 @@ Run all unit tests: cargo test --workspace --lib --bins ``` -`--bins` is not optional: `tablepro-app` has no `lib.rs`, so `--lib` alone skips every test in the app crate. `scripts/ci-local.sh` runs this command; the CI workflow still passes `--lib` only. +`--bins` is not optional: `tablepro-app` has no `lib.rs`, so `--lib` alone skips every test in the app crate. Both `scripts/ci-local.sh` and the CI workflow run this exact command. ## Integration tests @@ -138,7 +138,7 @@ What exists today is the driver-level smoke described above: `scripts/smoke-post GitHub Actions (`.github/workflows/build-linux.yml`), Ubuntu runner, two jobs: -1. **Fast checks**: `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, `cargo build --workspace`, `cargo test --workspace --lib`. Runs in an `ubuntu:25.10` container, which ships the glib version libadwaita 1.6 needs. `scripts/ci-local.sh` runs the same steps, but with `--lib --bins` so the app crate's tests actually run. The workflow should pick up `--bins` too. +1. **Fast checks**: `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, `cargo build --workspace`, `cargo test --workspace --lib --bins`. Runs in an `ubuntu:25.10` container, which ships the glib version libadwaita 1.6 needs. `scripts/ci-local.sh` runs the same steps with the same flags. 2. **Driver integration tests**: runs after fast checks pass. Boots Docker on the host runner and runs the Postgres, MySQL, and ClickHouse suites with `--include-ignored`. The MSSQL suite exists but is not wired in yet. PRs only merge when both jobs are green. diff --git a/linux/flatpak/com.tablepro.linux.json b/linux/flatpak/com.tablepro.linux.json index 9dde351f2..fcb047402 100644 --- a/linux/flatpak/com.tablepro.linux.json +++ b/linux/flatpak/com.tablepro.linux.json @@ -4,7 +4,8 @@ "runtime-version": "47", "sdk": "org.gnome.Sdk", "sdk-extensions": [ - "org.freedesktop.Sdk.Extension.rust-stable" + "org.freedesktop.Sdk.Extension.rust-stable", + "org.freedesktop.Sdk.Extension.llvm18" ], "command": "tablepro-app", "finish-args": [ @@ -14,12 +15,16 @@ "--socket=wayland", "--device=dri", "--filesystem=home", + "--filesystem=/etc/krb5.conf:ro", + "--filesystem=/run/.heim_org.h5l.kcm-socket", "--talk-name=org.freedesktop.secrets" ], "build-options": { - "append-path": "/usr/lib/sdk/rust-stable/bin", + "append-path": "/usr/lib/sdk/rust-stable/bin:/usr/lib/sdk/llvm18/bin", + "prepend-ld-library-path": "/usr/lib/sdk/llvm18/lib", "env": { - "CARGO_HOME": "/run/build/tablepro-app/cargo" + "CARGO_HOME": "/run/build/tablepro-app/cargo", + "LIBCLANG_PATH": "/usr/lib/sdk/llvm18/lib" } }, "modules": [ diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index a926c49a6..40f52763f 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -1,5 +1,6 @@ # Source files containing translatable strings. # Used by `xtr` (Rust xgettext) when regenerating po/tablepro.pot. +crates/app/src/services/connection_service.rs crates/app/src/ui/app.rs crates/app/src/ui/connect_dialog.rs crates/app/src/ui/edit_dialog.rs