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>
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