feat: support Windows/Kerberos integrated authentication - #25
Conversation
Enable the mssql-tds-preview crate's default sspi/gssapi features and map AuthMethod::Integrated when integrated_auth is set. Accept Integrated Security / Trusted_Connection in connection strings instead of hard-rejecting them, and reject the combination with a username/password instead of silently picking one. Also accept the Command Timeout keyword as a client-side no-op, matching the existing Connect Timeout handling. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for this, the connection-string handling and the I made some changes on top of your branch to move the UI part into the plugin instead of core: the "Use Windows Authentication" checkbox is now a plugin UI extension on the Tomorrow I will look at how best to integrate this with your work here (most likely as a follow-up on top of this PR) and I will keep you posted. A live SSPI re-test on your side once that lands would be very welcome. |
TabularisDB/tabularis#780 supersedes the host-side capability/field approach from #775 with a generic connection-modal.extra_fields hook (credentialFieldsHidden/setCredentialFieldsHidden) plus the existing opaque extra map, so no core schema change is needed. Rebuild this plugin's side on top of that: - Add ui/, a Vite+React IIFE bundle (per PLUGIN_GUIDE.md) contributing the "Use Windows Authentication" checkbox to connection-modal.extra_fields, gated to driver "sqlserver". It writes extra.integrated_auth and calls setCredentialFieldsHidden; degrades to a visible-but-unhidden checkbox on hosts without that hook. - ConnectionParams gains extra: HashMap<String, String>; resolve_connection_params now also resolves integrated_auth from extra["integrated_auth"] == "true", in addition to the existing Integrated Security=True connection-string path. Restructured the early-return so this works without a connection string. - Fix build_connection_key: it never folded auth mode into the pool cache key, so editing a saved connection between SQL and Windows auth could reuse a stale pool built under the previous credentials. - .tabularium: drop the now-unused supports_integrated_auth capability, add the ui_extensions entry for the new checkbox. - README: document the checkbox as a UI extension instead of a discrete connection field. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Four parallel reviews (code quality, test coverage, silent failures, comment accuracy) on the current diff surfaced two real correctness bugs and a CI gap, all fixed here: - resolve_connection_params: a connection-string Integrated Security=False silently overrode an extra["integrated_auth"]=true with no error, unlike every other dual-sourced field in this function (which reconcile/reject on conflict). Now raises a contradiction error instead. - extra["integrated_auth"] used a brittle exact match against the literal string "true", unlike this file's own parse_bool convention used for the same flag in connection strings (case-insensitive, hard-errors on garbage instead of silently defaulting to SQL auth). Now reuses parse_bool. - ui/ was never typechecked or built in CI — only in release.yml, so a TypeScript error would first surface at release time. Added a ci.yml job mirroring explain-package's pattern, and switched release.yml to `npm ci` since ui/package-lock.json is committed. - Added the missing test coverage the reviews identified: extra-map + connection-string agreeing/disagreeing on integrated_auth, and a driver/pool.rs test for the AuthMethod::Integrated branch. - Fixed two doc comments left inaccurate by earlier edits in this branch (pool_manager.rs's illustrative key layout and README wording). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Cut the ratio of prose to code introduced across this branch's edits. No behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Thanks for updating this! I tested the latest revision (9e2f030) locally. The 215 Rust/conformance tests and all 26 integration tests against a local SQL Server passed, as did clippy, formatting, and the UI typecheck/build. I also exercised integrated auth over JSON-RPC: it reaches GSSAPI and gives the expected I did find one UI issue: when reopening a saved connection with I also checked which nightly first includes TabularisDB/tabularis#780. It’s nightly-20260918-992d969, whose actual app version is 0.24.1-2. I verified both the commit ancestry and the updater’s To require the full credential-hiding/keychain behavior, let’s set Other than that, the checks I ran look good. Thanks again for the Windows testing! |
IntegratedAuthToggle only called setCredentialFieldsHidden from onChange, so reopening a saved connection with extra.integrated_auth=true left the username/password inputs visible. Move the call into an effect keyed on the flag so mount and load sync too. Also require Tabularis 0.24.1-2 (first nightly containing TabularisDB/tabularis#780) in .tabularium and the README, and add a vitest regression test wired into CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Thanks for the quick fix, @egertaia! I pulled the update and retested the saved-connection case against the built bundle — it now works as expected, and the toggle still behaves correctly in both directions. The new tests, typecheck and build pass too, and the runtime requirement is now correct. Thanks for sticking with this and for testing SSPI on Windows. I’ll merge this and include it in the next beta. |
Summary
Originally implemented
integrated_authas a discrete connection field plussupports_integrated_authcapability on core (matching #775). While testingend-to-end, @debba noted
that #775 was superseded by TabularisDB/tabularis#780,
a smaller host-side hook:
connection-modal.extra_fieldsgainscredentialFieldsHidden/setCredentialFieldsHidden, and the flag itselftravels through the host's existing opaque
extramap instead of a newcore field. This PR rebuilds the plugin side on top of that:
mssql-tds-preview's defaultsspi/gssapifeatures enabled (wasdefault-features = false) andAuthMethod::Integratedmapped whenintegrated_authis set.ui/: a Vite+React IIFE bundle (perPLUGIN_GUIDE.md's UIExtensions section) contributing the "Use Windows Authentication"
checkbox to
connection-modal.extra_fields, gated todriver: "sqlserver"via the manifest. Writes
extra.integrated_authand callssetCredentialFieldsHidden; degrades gracefully (checkbox works, logininputs just stay visible) on a host without that hook.
ConnectionParamsgainsextra: HashMap<String, String>;resolve_connection_paramsresolvesintegrated_authfromextra["integrated_auth"] == "true"or fromIntegrated Security=True/Trusted_Connection=Trueinconnection_string— either source rejects acombined username/password.
build_connection_key: it never folded auth mode into the poolcache key, so editing a saved connection between SQL and Windows auth
could reuse a stale pool built under the previous credentials.
.tabularium: dropped the now-unusedsupports_integrated_authcapability, added the
ui_extensionsentry.connection field; also accepts the
Command Timeoutkeyword as aclient-side no-op (needed for real ADO.NET/SSMS-style connection strings).
Companion host PR: TabularisDB/tabularis#780
Test plan
cargo test— 213 tests passing (connection.rs,pool_manager.rs,driver/pool.rs)cargo clippy --all-targets -- -D warnings— cleancargo fmt --check— cleanui/:npm run typecheckandnpm run build— clean, produces a valid IIFE bundle exposing__tabularis_plugin__pnpm tauri devon #780's branch + this plugin installed): checkbox appears from the plugin's own UI extension, connects via SSPI with no username/password, same as SSMS