[egertaia]

Summary

This PR adds NTLM authentication. The user enters DOMAIN\user (or user@domain) and a password.

NTLM works on every platform. On Linux and macOS, it does not need Kerberos, kinit, or a system library. Today, integrated authentication on those platforms uses only Kerberos, through GSSAPI.

The NTLM provider uses the SecurityContextProvider extension point from microsoft/mssql-rs#742 (issue microsoft/mssql-rs#707). The bridge passes the provider through Config::security_context_provider. That method was added in saurabh500/mssql-tiberius-bridge#146 and released in version 0.3.0.

Dependencies

This PR uses the published releases: mssql-tds 0.4.0 and mssql-tiberius-bridge 0.3.0.

Do not merge Dependabot #56 on its own. It upgrades mssql-tds only. The bridge pins its own mssql-tds, so the build breaks. This PR replaces #56.

Changes

• src/driver/ntlm.rs adds NtlmProvider. It implements the mssql-tds SecurityContextProvider trait with the pure-Rust sspi NTLM client. The crate is used with default-features = false.
• A new extra.ntlm flag works like extra.integrated_auth. NTLM uses the username and password fields. NTLM requires a username. NTLM and integrated_auth cannot both be set.
• The pool key includes an ntlm:<user> mode. A saved connection that changes between SQL authentication and NTLM gets its own pool.
• The connection modal adds a checkbox, "Use NTLM (username as DOMAIN\user)". It appears next to "Use Windows Authentication". Each checkbox clears the other. NTLM keeps the username and password fields visible.
• Upgrades mssql-tiberius-bridge to 0.3.0 and mssql-tds to 0.4.0.
• Maps two new mssql-tds 0.4.0 errors to driver failures: EncodedValueTooLong and TvpCellSerialization.
• Updates the README and CHANGELOG.

Limitation: The NTLM provider does not send channel bindings (Extended Protection). A server that requires Extended Protection rejects NTLM. The README states this.

Validation

• cargo test: 230 unit tests pass. One test runs a full NTLM handshake between NtlmProvider and the sspi server. The handshake succeeds with the correct password and fails with a wrong one.
• cargo clippy --all-targets -- -D warnings reports no warnings. cargo fmt --all -- --check reports no changes.
• UI: tsc --noEmit passes. Five vitest tests pass, three of them new. vite build passes.
• markdownlint-cli2 passes on the README and CHANGELOG.

Not run:

• Live SQL Server tests. They need a running server. The author's ARM Mac cannot start the SQL Server image.
• NTLM against a real domain and SQL Server.

🤖 Generated with Claude Code

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The incremental change since the last review (aa59a34..4d5189b) is a single-line edit to .github/workflows/ci.yml.

• The rustsec/audit-check ignore input changes from a space-separated list to a comma-separated list (RUSTSEC-2026-0235,RUSTSEC-2023-0071). The action documents the ignore input as comma-separated, so the previous space-separated value would likely have been read as one unrecognised ID and left both advisories unignored. This change fixes that.
• The explanatory comments above the input are unchanged and still accurate.

Not verified: I did not run CI or cargo audit, and the comma-separated format comes from my knowledge of the action's documented input, which I did not re-check against its source.

Files Reviewed (1 file in this increment)

• .github/workflows/ci.yml

Previous Review Summaries (5 snapshots, latest commit aa59a34)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit aa59a34)

Status: No Issues Found | Recommendation: Merge

The incremental change since the last review (e8a3b10..aa59a34) is a single edit to .github/workflows/ci.yml.

• The cargo-audit ignore list now also covers RUSTSEC-2023-0071 (Marvin timing side channel in rsa), with a comment explaining why.
• The stated rationale is that sspi pulls in picky and rsa with no feature gate, and the NTLM path performs no RSA private-key operations. There is no patched release to upgrade to.
• The comment records when to re-check: when sspi drops picky, or rsa ships a fix.
• The ignore is scoped to one advisory ID, so other advisories are still reported.

Not verified: I did not run CI, cargo audit or the tests, and I did not confirm the dependency graph claim with cargo tree.

Files Reviewed (1 file in this increment)

• .github/workflows/ci.yml

Previous review (commit e8a3b10)

Status: No Issues Found | Recommendation: Merge

The incremental change since the last review (5546011..e8a3b10) is a small refactor of ui/src/IntegratedAuthToggle.tsx. Behavior is unchanged.

• The two duplicated checkbox blocks are replaced by a shared ModeCheckbox component, and the inline labelStyle constant is inlined.
• select is renamed selectMode and typed with an AuthMode union.
• The mutual-exclusion logic is the same: turning one mode on clears the other.
• The useEffect still hides the credential fields for integrated_auth only, as before.
• The DOMAIN\user label is passed as a JSX string attribute, so the backslash renders literally, as it did before.

Not verified: I did not run the UI or the tests, and I did not test live NTLM against a real SQL Server or domain.

Files Reviewed (1 file in this increment)

• ui/src/IntegratedAuthToggle.tsx

Previous review (commit 5546011)

Status: No Issues Found | Recommendation: Merge

The incremental changes since the last review (87d9dde..5546011) are test-only. They move the random_secret() helper into src/common.rs as a #[cfg(test)] function and use it in place of the remaining hardcoded passwords.

• random_secret() is test-only and rand is a dev-dependency, so release builds are unaffected.
• The helper is used from connection.rs, driver/error.rs, driver/ntlm.rs, driver/pool/tests.rs and rpc.rs. common is declared only in main.rs, and src/bin/test_plugin.rs does not use it, so the shared helper resolves everywhere it is needed.
• The credential-redaction and parse-error tests still check what they did before. They now build their strings from random alphanumeric secrets, and the %ZZ malformed-escape case is still malformed.

Not verified: live NTLM against a real SQL Server or domain. I also did not run the tests.

Files Reviewed (6 files in this increment)

• src/common.rs
• src/connection.rs
• src/driver/error.rs
• src/driver/ntlm.rs
• src/driver/pool/tests.rs
• src/rpc.rs

Previous review (commit 87d9dde)

Status: No Issues Found | Recommendation: Merge

The incremental changes since the last review (54c59de..87d9dde) are test-only. They replace hardcoded NTLM test passwords with random secrets, which addresses the reviewer's request.

• rand 0.8 is added as a dev-dependency only, so it does not affect release builds. The Cargo.lock entry matches.
• random_secret() uses the correct rand 0.8 APIs (thread_rng, Alphanumeric, sample_iter).
• The wrong-password test asserts the two secrets differ, which keeps it deterministic in practice (24 alphanumeric characters).

Not verified: live NTLM against a real SQL Server or domain.

Files Reviewed (3 files in this increment)

• Cargo.toml
• Cargo.lock
• src/driver/ntlm.rs

Previous review (commit 54c59de)

Status: No Issues Found | Recommendation: Merge

No defects were found in the changed code. Observations (not blocking):

• Cargo.lock now matches the mssql-tds 0.4.0 / mssql-tiberius-bridge 0.3.0 pins, so the "CI fails until released" note in the PR description appears to be resolved by the second commit.
• NtlmProvider is only built after resolve_connection_params has validated ntlm. build_config re-resolves, so extra.ntlm is honored on every path.
• The pool key does not include the password, which is a documented existing gap. Changing the password on a saved NTLM connection will reuse the old pool, as it already does for SQL auth.
• Not verified: live NTLM against a real SQL Server or domain, which the PR description also notes was not run.

Files Reviewed (14 files)

• CHANGELOG.md
• Cargo.lock
• Cargo.toml
• README.md
• src/connection.rs
• src/driver/error.rs
• src/driver/mod.rs
• src/driver/ntlm.rs
• src/driver/pool.rs
• src/driver/pool/tests.rs
• src/models.rs
• src/pool_manager.rs
• ui/src/IntegratedAuthToggle.test.tsx
• ui/src/IntegratedAuthToggle.tsx

---

_{Reviewed by claude-sonnet-5.5 · Input: 252 · Output: 1.3K · Cached: 165.5K}

[egertaia]

@saurabh500 Please review and approve when you have time. All review comments are answered. The latest commits are 87d9dde and 5546011.

Status:

• Build uses the published mssql-tds 0.4.0 and mssql-tiberius-bridge 0.3.0.
• Unit tests pass. Clippy and fmt are clean.
• Live database tests need a SQL Server. CI must run them.

[saurabh500]

@debba the Version suggestion step is failing, since this is a forked PR. I am tempted to merge this PR since I dont expect a side effect of the failing GH action. But wanted to get your opinion.