Skip to content

feat: support Windows/Kerberos integrated authentication - #25

Merged
debba merged 6 commits into
TabularisDB:mainfrom
egertaia:feat/windows-integrated-auth
Sep 21, 2026
Merged

debba merged 6 commits into
TabularisDB:mainfrom
egertaia:feat/windows-integrated-auth

Conversation

@egertaia

@egertaia egertaia commented Sep 16, 2026

Copy link
Copy Markdown

Summary

Originally implemented integrated_auth as a discrete connection field plus
supports_integrated_auth capability on core (matching #775). While testing
end-to-end, @debba noted
that #775 was superseded by TabularisDB/tabularis#780,
a smaller host-side hook: connection-modal.extra_fields gains
credentialFieldsHidden / setCredentialFieldsHidden, and the flag itself
travels through the host's existing opaque extra map instead of a new
core field. This PR rebuilds the plugin side on top of that:

  • mssql-tds-preview's default sspi/gssapi features enabled (was
    default-features = false) and AuthMethod::Integrated mapped when
    integrated_auth is set.
  • New ui/: a Vite+React IIFE bundle (per PLUGIN_GUIDE.md's UI
    Extensions section) contributing the "Use Windows Authentication"
    checkbox to connection-modal.extra_fields, gated to driver: "sqlserver"
    via the manifest. Writes extra.integrated_auth and calls
    setCredentialFieldsHidden; degrades gracefully (checkbox works, login
    inputs just stay visible) on a host without that hook.
  • ConnectionParams gains extra: HashMap<String, String>;
    resolve_connection_params resolves integrated_auth from
    extra["integrated_auth"] == "true" or from Integrated Security=True /
    Trusted_Connection=True in connection_string — either source rejects a
    combined username/password.
  • Fixed 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: dropped the now-unused supports_integrated_auth
    capability, added the ui_extensions entry.
  • README: documents the checkbox as a UI extension instead of a discrete
    connection field; also accepts the Command Timeout keyword as a
    client-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 — clean
  • cargo fmt --check — clean
  • ui/: npm run typecheck and npm run build — clean, produces a valid IIFE bundle exposing __tabularis_plugin__
  • Manually verified end-to-end against a real domain-joined SQL Server via a Windows build (pnpm tauri dev on #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
  • Not covered by CI: Linux/macOS GSSAPI path (needs a Kerberos KDC; only SSPI was live-tested)

Egert Aia and others added 2 commits September 16, 2026 15:14
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>
@debba

debba commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Thanks for this, the connection-string handling and the mssql-tds feature switch are exactly what was needed.

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 connection-modal.extra_fields slot, the flag is read from the host's opaque extra map (extra.integrated_auth) as well as from Integrated Security=True in connection strings, and the supports_integrated_auth capability goes away. The matching host change is TabularisDB/tabularis#780.

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.

Egert Aia and others added 3 commits September 17, 2026 10:04
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>
@debba

debba commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

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 kinit error without a ticket, and correctly rejects a username or password alongside integrated auth. I couldn’t verify a successful Kerberos login or SSPI on Windows myself.

I did find one UI issue: when reopening a saved connection with extra.integrated_auth=true, the checkbox is checked but the username/password fields are still visible. I reproduced this with a small React test against the built UI bundle. IntegratedAuthToggle only calls setCredentialFieldsHidden in onChange, so we also need to sync that state when the saved value is loaded, without requiring another click.

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 latest.json. The September 17 nightly doesn’t include it, despite being published after the merge, because it was built from an earlier commit. Stable 0.24.0 doesn’t include it either.

To require the full credential-hiding/keychain behavior, let’s set min_runtime_version in .tabularium to "0.24.1-2" and update the requirement in the README. It needs to be the app’s SemVer, not the nightly tag; using 0.24.1 would exclude those nightlies too.

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

debba commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

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.

@debba debba added the prerelease:beta Publish the next release on the beta prerelease channel label Sep 21, 2026
@debba
debba merged commit bee2ac5 into TabularisDB:main Sep 21, 2026
15 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

prerelease:beta Publish the next release on the beta prerelease channel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants