Add ActiveDirectoryInteractive auth to mssql-odbc via mssql-auth (T3)

[Vahid (Vahid-b)]

Description

Implements T3: Authentication=ActiveDirectoryInteractive for mssql-odbc, matching the classic C++ msodbcsql driver.

⚠️ This PR was redesigned after review — please re-read

The earlier revision hand-rolled an OAuth2 authorization-code + PKCE flow over a loopback listener and the system browser. That design has been removed. Interactive sign-in is now delegated to mssql-auth.dll (OneAuth/MSQA) — the same library msodbcsql loads.

auth/interactive.rs is rewritten, auth/msqa.rs is new, and the loopback/PKCE/browser-launch code, its dependencies (base64, getrandom, sha2, serde, serde_json) and its tests are gone. Most of the earlier review discussion (PKCE, state validation, redirect classification, browser launching, zombie reaping) no longer applies to any code in this PR.

Why the design changed

mssql-odbc presents msodbcsql's own client id. The two drivers therefore have to be indistinguishable to callers and to the service — a keyword that behaved differently between them would be a behavioural fork, not parity. Delegating to the same auth library yields identical prompts, identical caching semantics, and identical error text, and avoids this repo owning a hand-rolled OAuth implementation.

Platform behaviour

Platform	Behaviour
Windows	Interactive sign-in via mssql-auth.dll
Linux / macOS	Resolves to ActiveDirectoryIntegrated (msodbcsql's own fall-through)

msodbcsql's interactive support is Windows-only. From the C++ source:

• SNI_FedAuth.hpp:16 wraps the entire header in #if !defined(MPLAT_BUILD) //No MSQA for non-windows clients
• sni/src/Makefile omits SNI_FedAuth from SRCLIST — it is never compiled on Unix
• SNI_FedAuth.cpp:249 loads the DLL via LoadLibraryExA; the repo contains no dlopen counterpart

On Unix the MSQA branch and its else arm both sit inside #if !defined(XPLAT_ODBC_TODO) (Parse.cpp:3597-3660), so the guard removes both and control falls through to AzureADAuth with AKVCFG_AUTHMODE_INTEGRATED. Nothing rejects the keyword earlier (sqlcconn.cpp:4066-4078 accepts it on every platform).

An explicit "interactive is not supported on this platform" error was prototyped and deliberately withdrawn: because we present msodbcsql's client id, refusing a keyword msodbcsql accepts would be the one place the drivers visibly disagree. Improving on the fall-through is tracked separately rather than shipped as a silent divergence.

How it works

InteractiveTokenFactory implements the existing mssql-tds EntraIdTokenFactory seam, so it plugs into the FedAuth handshake exactly like the T2 service-principal factory. auth/msqa.rs holds the mssql-auth.dll bindings and mirrors SNI_FedAuth.cpp:

• Authentication contexts are cached on username + "\0" + stsUrl (SNI_FedAuth.cpp:127), created with ForcePrompt=TRUE, and ForcePrompt is cleared after the first success — so a second connection reuses the context and does not re-prompt.
• MSQAGetRequestStatus decides whether interaction is needed; when it is, the window is hosted on a dedicated STA thread (CoInitializeEx(COINIT_APARTMENTTHREADED) + MSQAUICreateHostWindow + message pump), and the completion callback ends the pump via PostThreadMessage(WM_QUIT).
• MSQAReleaseAuthenticationContext is intentionally not called for interactive (SNI_FedAuth.cpp:777-782) — the context is the token cache.
• OneAuth packs its Status into the high byte of the returned error. Only NetworkTemporarilyUnavailable, ServerTemporarilyUnavailable and TransientError are retryable, mirroring IsTransientError (:301-306). UserCanceled is not transient, so a cancelled sign-in fails once instead of reopening the window.

Parity parameters, all from Parse.cpp:3601-3620:

Parameter	Value
Client id	2c1229aa-16c5-4ff5-b46b-4f7fe2a2a9c8 (msodbcsql's ODBC client id)
Redirect URI	https://sqlaad/
WAM	off by default (ADALuseWAM defaults to 0, sqlcconn.cpp:3417)
Window title	"Authenticate to database on %s" (IDS_AD_AUTH_DB)

Login timeout

Also lands SQL_ATTR_LOGIN_TIMEOUT and a real SQLGetConnectAttrW. mssql-tds previously had a single connect_timeout bounding both the overall login deadline and each TCP connect, so allowing minutes for an MFA prompt also let an unreachable host hang that long. These are now split, matching msodbcsql:

• connect_timeout (default 15s) — per-TCP-connect cap; unreachable servers still fail fast.
• login_timeout: Option<u32> (new) — overall login deadline. None falls back to connect_timeout, so existing callers are unaffected; Some(0) waits indefinitely.

Interactive installs a 330s default only when the app has not set its own SQL_ATTR_LOGIN_TIMEOUT. If the deadline expires while a window is open, CancelUiOnDrop posts WM_QUIT so the window is torn down rather than orphaned.

Tests

cargo test -p mssql-odbc --lib → 419 pass. Unit tests cover context-cache keying, transient/permanent status classification, error-description reads, configure_auth dispatch on both platforms, and the login-timeout attribute (including 0 → infinite and preserving an app-set value).

Because Linux/macOS cannot be cross-compiled here (openssl-sys), the non-Windows path was validated by inverting every cfg(windows)/cfg_attr gate and building: clippy clean under -D warnings, 333 tests pass. That caught a real break — an unused-variable error that would only have appeared in Linux CI.

Live verification

The sign-in window and token read cannot run in CI, so this was verified manually against a live Azure SQL server and Entra tenant:

Check	Result
Interactive sign-in completes	SQL_SUCCESS_WITH_INFO; SUSER_NAME() returns the Entra account
Second connection does not re-prompt	0.1s, no window (vs 19.4s with the prompt) — context cache confirmed
Cancelling does not relaunch the prompt	exactly one window; SQLSTATE 08001, one failure
SQL_ATTR_LOGIN_TIMEOUT round-trips	set 300 → read back 300

Known limitations

• mssql-auth.dll is not redistributed by this driver. Interactive works where msodbcsql 18 has already installed it into System32. Packaging is unresolved and needs a decision before GA.
• A slow first sign-in can lose the connection. TDS FedAuth requires the server's FEDAUTHINFO (carrying the STS URL) before a token can be acquired, so the connection idles for the whole sign-in; a multi-minute first-time MFA can exceed the Azure SQL gateway's idle timeout. The token is acquired successfully — the failure surfaces from the TDS login parser. msodbcsql prompts from the same point in the login exchange and shares this exposure, so it is treated as parity, not a defect here.
• Token caching is per-process, matching msodbcsql — nothing is persisted to disk. Cross-connection caching is tracked in AB#46409.

Related Issues

AB#46067

Checklist

• cargo bfmt passes
• cargo bclippy passes
• cargo btest passes — cargo test -p mssql-odbc --lib (419 pass) locally; nextest/llvm-cov unavailable on this machine, so full coverage runs in CI
• New/changed functionality has tests
• Public API changes are documented

[Copilot]

Pull request overview

Adds browser-based Active Directory Interactive authentication to the ODBC driver using OAuth2 authorization code flow with PKCE.

Changes:

• Implements browser launch, loopback redirect handling, token exchange, and caching.
• Registers interactive authentication with the TDS FedAuth pipeline.
• Adds dependencies and unit tests for PKCE, redirects, URLs, and token parsing.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
mssql-odbc/src/auth/mod.rs	Registers the interactive authentication module.
mssql-odbc/src/auth/interactive.rs	Implements and tests the interactive OAuth2 flow.
mssql-odbc/src/auth/entra.rs	Configures the interactive token factory and shared helpers.
mssql-odbc/src/api/driver_connect.rs	Updates authentication support and unsupported-method testing.
mssql-odbc/Cargo.toml	Adds networking, PKCE, randomness, and JSON dependencies.

[github-actions]

📊 Code Coverage Report

🔥 Diff Coverage 67%

🎯 Overall Coverage 91.0%

📦 Project: mssql-tds + mssql-odbc + mssql-py-core ℹ️ Note: diff coverage is reported, not enforced.

---

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

• mssql-odbc/src/api/driver_connect.rs (84.6%): Missing lines 263,282
• mssql-odbc/src/api/exports.rs (0.0%): Missing lines 621-626
• mssql-odbc/src/api/get_connect_attr.rs (95.4%): Missing lines 70-71,92-93,117-119
• mssql-odbc/src/api/set_connect_attr.rs (100%)
• mssql-odbc/src/auth/entra.rs (100%)
• mssql-odbc/src/auth/interactive.rs (74.8%): Missing lines 91,94-95,100,102-107,113,115-118,120,125-128,131,133-134,159,162,236
• mssql-odbc/src/auth/msqa.rs (31.0%): Missing lines 201-203,219-223,226-227,229,237-242,246-253,256-257,261,271,277,281-287,289,302-313,315-318,320-323,325-331,333-338,340-343,345-351,359-366,378-385,389,391-396,398-403,405,407-410,412-413,416-418,422-424,433-444,446-451,456-460,464-465,467-468,471,480-485,487-489,495-499,501-502,504-515,518,521,523,530-533,537-543,546-547,554-558,575-578,588-595,597-603,605-609,614-617,619-623,625-632,636,638-639,644-645,647-650
• mssql-odbc/src/handles/dbc.rs (80.0%): Missing lines 94
• mssql-tds/src/connection/client_context.rs (100%)
• mssql-tds/src/connection_provider/tds_connection_provider.rs (90.6%): Missing lines 151,199,203
• mssql-tds/src/error/mod.rs (100%)

Summary

• Total: 902 lines
• Missing: 296 lines
• Coverage: 67%

mssql-odbc/src/api/driver_connect.rs

  259     // Apply an app-set SQL_ATTR_LOGIN_TIMEOUT before configuring auth so an
  260     // explicit login timeout takes precedence over any method-specific default
  261     // (e.g. the larger default interactive sign-in installs).
  262     if let Some(secs) = state.login_timeout {
! 263         context.login_timeout = Some(secs);
  264     }
  265 
  266     if let Err(unsupported) = configure_auth(&mut context, resolved, &params.server) {
  267         let UnsupportedAuth {

---

  278         // method the connection string never mentioned.
  279         let message = if requested == resolved {
  280             format!("Authentication method {requested:?} is not yet supported")
  281         } else {
! 282             format!(
  283                 "Authentication method {requested:?} resolves to {resolved:?} on this platform, \
  284                  which is not yet supported"
  285             )
  286         };

---

mssql-odbc/src/api/exports.rs

  617     string_length_ptr: *mut SqlInteger,
  618 ) -> SqlReturn {
  619     crate::init_tracing();
  620     unsafe {
! 621         super::get_connect_attr::sql_get_connect_attr_w(
! 622             connection_handle,
! 623             attribute,
! 624             value_ptr,
! 625             buffer_length,
! 626             string_length_ptr,
  627         )
  628     }
  629 }

---

mssql-odbc/src/api/get_connect_attr.rs

  66     buffer_length: SqlInteger,
  67     string_length_ptr: *mut SqlInteger,
  68 ) -> SqlReturn {
  69     if connection_handle.is_null() {
! 70         error!("SQLGetConnectAttrW: connection_handle is null");
! 71         return SQL_INVALID_HANDLE;
  72     }
  73 
  74     let dbc = unsafe { handle_from_raw::<DbcHandle>(connection_handle) };
  75     debug_assert_eq!(

---

  88     _buffer_length: SqlInteger,
  89     _string_length_ptr: *mut SqlInteger,
  90 ) -> SqlReturn {
  91     let Ok(mut state) = dbc.inner.lock() else {
! 92         error!("SQLGetConnectAttrW: dbc mutex poisoned");
! 93         return SQL_ERROR;
  94     };
  95     free_errors(&mut state);
  96 
  97     match attribute {

---

  113         // would make a set/get round-trip fail for a value the driver had just
  114         // reported as accepted.
  115         SQL_ATTR_ACCESS_MODE | SQL_ATTR_CONNECTION_TIMEOUT | SQL_ATTR_PACKET_SIZE => {
  116             if value_ptr.is_null() {
! 117                 error!(attribute, "SQLGetConnectAttrW: value pointer is null");
! 118                 post_diag(&mut state, ERR_INVALID_NULL_POINTER);
! 119                 return SQL_ERROR;
  120             }
  121             let value = match attribute {
  122                 SQL_ATTR_ACCESS_MODE => state.access_mode,
  123                 SQL_ATTR_CONNECTION_TIMEOUT => state.connection_timeout,

---

mssql-odbc/src/auth/interactive.rs

   87     /// sign-in lock can be taken here, where waiting *is* cancellable. Taking it
   88     /// inside the acquisition instead would park a blocking-pool thread behind
   89     /// whoever is signing in, for the full length of a human sign-in, even after
   90     /// this connection's deadline had passed.
!  91     async fn acquire(&self, spn: &str, sts_url: &str) -> TdsResult<String> {
   92         // msodbcsql titles the window "Authenticate to database on %s" with the
   93         // server name (`local.rc:786`, applied at `Parse.cpp:3618-3619`).
!  94         let window_title = format!("Authenticate to database on {}", self.server);
!  95         debug!(
   96             server = %self.server,
   97             "interactive: acquiring an Entra token via mssql-auth"
   98         );
   99 
! 100         let spn = spn.to_string();
  101 
! 102         let context = {
! 103             let (sts_url, login_hint) = (sts_url.to_string(), self.login_hint.clone());
! 104             spawn_acquisition(move || {
! 105                 super::msqa::resolve_context(&sts_url, &login_hint, PUBLIC_CLIENT_ID, REDIRECT_URI)
! 106             })
! 107             .await?
  108         };
  109 
  110         // One sign-in window per account at a time. Awaited rather than blocked
  111         // on, so a caller that hits its login deadline while another connection

---

  109 
  110         // One sign-in window per account at a time. Awaited rather than blocked
  111         // on, so a caller that hits its login deadline while another connection
  112         // is signing in leaves the queue instead of occupying a blocking thread.
! 113         let serialized = Arc::clone(&context.sign_in_lock).lock_owned().await;
  114 
! 115         let ui_thread_id = Arc::new(super::msqa::UiThreadId::default());
! 116         let cancel_on_drop = CancelUiOnDrop {
! 117             ui_thread_id: Some(Arc::clone(&ui_thread_id)),
! 118         };
  119 
! 120         let result = spawn_acquisition(move || {
  121             // Held inside the blocking task rather than by the caller: a
  122             // dropped future would otherwise release the lock while this
  123             // acquisition was still tearing its window down, letting the next
  124             // waiter open a second prompt on top of it.
! 125             let _serialized = serialized;
! 126             super::msqa::acquire_token(&context, &spn, &window_title, &ui_thread_id)
! 127         })
! 128         .await;
  129 
  130         // The sign-in finished on its own, so there is no window to close.
! 131         cancel_on_drop.disarm();
  132 
! 133         result
! 134     }
  135 }
  136 
  137 /// Runs one step of the acquisition on a blocking thread, flattening a join
  138 /// failure into a connection error.

---

  155         &self,
  156         spn: String,
  157         sts_url: String,
  158         _auth_method: TdsAuthenticationMethod,
! 159     ) -> TdsResult<Vec<u8>> {
  160         let token = self.acquire(&spn, &sts_url).await?;
  161         Ok(super::entra::encode_utf16le(&token))
! 162     }
  163 }
  164 
  165 /// Closes an in-flight sign-in window if the acquisition future is dropped —
  166 /// which is what happens when the connection's login deadline expires.

---

  232         match result {
  233             Err(Error::ConnectionError(message)) => {
  234                 assert!(message.contains("did not run to completion"), "{message}");
  235             }
! 236             other => panic!("expected a connection error, got {other:?}"),
  237         }
  238     }
  239 
  240     #[test]

---

mssql-odbc/src/auth/msqa.rs

  197     request: HMsqaRequest,
  198 }
  199 
  200 impl Drop for RequestGuard {
! 201     fn drop(&mut self) {
! 202         unsafe { (self.api.delete_request)(self.request) };
! 203     }
  204 }
  205 
  206 /// Contexts cached by `(login hint, STS URL)`, matching msodbcsql's cache key.
  207 type ContextCache = Mutex<HashMap<(String, String), Arc<CachedContext>>>;

---

  215 /// process that ran once without the Microsoft SQL Server authentication
  216 /// library installed could never use interactive auth again, even after the
  217 /// user installs msodbcsql 18 and retries — the retry would replay the
  218 /// remembered error without touching the filesystem.
! 219 fn api() -> TdsResult<&'static MsqaApi> {
! 220     if let Some(api) = MSQA_API.get() {
! 221         return Ok(api);
! 222     }
! 223     match load_api() {
  224         // A concurrent caller may have won the race; its table is equivalent,
  225         // and the extra `LoadLibraryExA` reference is deliberately never freed.
! 226         Ok(api) => Ok(MSQA_API.get_or_init(|| api)),
! 227         Err(message) => Err(Error::Security(SecurityError::LoadLibraryFailed(message))),
  228     }
! 229 }
  230 
  231 /// Loads the library and binds its entry points.
  232 ///
  233 /// The search is restricted to System32 (as msodbcsql does at

---

  233 /// The search is restricted to System32 (as msodbcsql does at
  234 /// `SNI_FedAuth.cpp:249`) so a DLL dropped next to the application cannot
  235 /// hijack authentication. The module is never freed: OneAuth spins up
  236 /// background state that must outlive individual connections.
! 237 fn load_api() -> Result<MsqaApi, String> {
! 238     let name = CString::new(MSQA_LIBRARY).expect("library name has no interior NUL");
! 239     let module = unsafe {
! 240         LoadLibraryExA(
! 241             PCSTR(name.as_ptr().cast()),
! 242             None,
  243             LOAD_LIBRARY_SEARCH_SYSTEM32,
  244         )
  245     };
! 246     let module = match module {
! 247         Ok(m) => m,
! 248         Err(e) => {
! 249             return Err(format!(
! 250                 "{MSQA_LIBRARY} could not be loaded from the system directory ({e}). \
! 251                  Entra interactive authentication requires the Microsoft SQL Server \
! 252                  authentication library to be installed."
! 253             ));
  254         }
  255     };
! 256     resolve(module)
! 257 }
  258 
  259 /// Resolves every entry point the interactive flow needs, failing if any is
  260 /// missing rather than discovering the gap mid-sign-in.
! 261 fn resolve(module: HMODULE) -> Result<MsqaApi, String> {
  262     /// Looks up one export and transmutes it to its typed signature.
  263     ///
  264     /// `GetProcAddress` hands back an untyped code pointer; the transmute is
  265     /// sound because the signatures are transcribed from `msqa_api.h` and the

---

  267     macro_rules! entry {
  268         ($name:literal, $ty:ty) => {{
  269             let symbol = concat!($name, "\0");
  270             let address = unsafe { GetProcAddress(module, PCSTR(symbol.as_ptr())) }
! 271                 .ok_or_else(|| format!("{MSQA_LIBRARY} does not export {}", $name))?;
  272             unsafe { std::mem::transmute::<Farproc, $ty>(address) }
  273         }};
  274     }

---

  273         }};
  274     }
  275 
  276     Ok(MsqaApi {
! 277         create_context: entry!(
  278             "MSQACreateAuthenticationContext",
  279             PfnCreateAuthenticationContext
  280         ),
! 281         set_option: entry!("MSQASetOption", PfnSetOption),
! 282         acquire_token: entry!("MSQAAcquireToken", PfnAcquireToken),
! 283         get_request_status: entry!("MSQAGetRequestStatus", PfnGetRequestStatus),
! 284         get_access_token: entry!("MSQAGetAccessToken", PfnGetAccessToken),
! 285         get_error_description: entry!("MSQAGetErrorDescription", PfnGetErrorDescription),
! 286         ui_create_host_window: entry!("MSQAUICreateHostWindow", PfnUiCreateHostWindow),
! 287         delete_request: entry!("MSQADeleteRequest", PfnDeleteRequest),
  288     })
! 289 }
  290 
  291 /// Returns the cached context for `(login_hint, sts_url)`, creating it on first
  292 /// use.
  293 ///

---

  298 /// been obtained. Mirrors `MSQAAuthContextCache::getOrCreate`.
  299 ///
  300 /// An empty `login_hint` keys every account-less connection to the same entry;
  301 /// see the module-level "Known limitation" note.
! 302 fn get_or_create_context(
! 303     api: &'static MsqaApi,
! 304     sts_url: &str,
! 305     login_hint: &str,
! 306     client_id: &str,
! 307     redirect_uri: &str,
! 308 ) -> TdsResult<Arc<CachedContext>> {
! 309     let key = (login_hint.to_string(), sts_url.to_string());
! 310     let cache = CONTEXT_CACHE.get_or_init(|| Mutex::new(HashMap::new()));
! 311     let mut cache = cache
! 312         .lock()
! 313         .unwrap_or_else(|poisoned| poisoned.into_inner());
  314 
! 315     if let Some(existing) = cache.get(&key) {
! 316         debug!("interactive: reusing cached OneAuth authentication context");
! 317         return Ok(Arc::clone(existing));
! 318     }
  319 
! 320     let sts = to_c_string(sts_url, "STS URL")?;
! 321     let client = to_c_string(client_id, "client id")?;
! 322     let redirect = to_c_string(redirect_uri, "redirect URI")?;
! 323     let user = to_c_string(login_hint, "user name")?;
  324 
! 325     let handle = unsafe {
! 326         (api.create_context)(
! 327             PCSTR(sts.as_ptr().cast()),
! 328             PCSTR(client.as_ptr().cast()),
! 329             PCSTR(redirect.as_ptr().cast()),
! 330             PCSTR(user.as_ptr().cast()),
! 331         )
  332     };
! 333     if handle.is_null() {
! 334         let code = unsafe { GetLastError() }.0;
! 335         return Err(Error::Security(SecurityError::InternalError(format!(
! 336             "MSQACreateAuthenticationContext failed (Windows error {code})"
! 337         ))));
! 338     }
  339 
! 340     unsafe {
! 341         (api.set_option)(handle, MSQA_OPTION_FORCE_PROMPT, 1);
! 342         (api.set_option)(handle, MSQA_OPTION_USE_WAM, USE_WAM);
! 343     }
  344 
! 345     let context = Arc::new(CachedContext {
! 346         handle: ContextHandle(handle),
! 347         sign_in_lock: Arc::new(AsyncMutex::new(())),
! 348     });
! 349     cache.insert(key, Arc::clone(&context));
! 350     Ok(context)
! 351 }
  352 
  353 /// Loads `mssql-auth.dll` if needed and resolves the cached context for an
  354 /// account. Blocking: run it on a thread that may block.
  355 ///

---

  355 ///
  356 /// Split out from [`acquire_token`] so the caller can take
  357 /// [`CachedContext::sign_in_lock`] on the async side, where waiting is
  358 /// cancellable.
! 359 pub(super) fn resolve_context(
! 360     sts_url: &str,
! 361     login_hint: &str,
! 362     client_id: &str,
! 363     redirect_uri: &str,
! 364 ) -> TdsResult<Arc<CachedContext>> {
! 365     get_or_create_context(api()?, sts_url, login_hint, client_id, redirect_uri)
! 366 }
  367 
  368 /// Acquires an access token interactively. Blocking: run it on a thread that
  369 /// may block.
  370 ///

---

  374 ///
  375 /// `ui_thread_id` receives the id of the message-pump thread as soon as it
  376 /// starts, so a caller whose login deadline expires can tear the sign-in window
  377 /// down via [`cancel_ui`] instead of leaving it orphaned on screen.
! 378 pub(super) fn acquire_token(
! 379     context: &CachedContext,
! 380     resource: &str,
! 381     window_title: &str,
! 382     ui_thread_id: &Arc<UiThreadId>,
! 383 ) -> TdsResult<String> {
! 384     let api = api()?;
! 385     let resource_c = to_c_string(resource, "resource")?;
  386 
  387     // A fresh correlation id per acquisition, so a failed sign-in can be
  388     // located in the tenant's Entra sign-in logs.
! 389     let correlation_id = GUID::new().unwrap_or(GUID::zeroed());
  390 
! 391     let request = unsafe {
! 392         (api.acquire_token)(
! 393             context.handle.0,
! 394             PCSTR(resource_c.as_ptr().cast()),
! 395             &correlation_id,
! 396         )
  397     };
! 398     if request.is_null() {
! 399         let code = unsafe { GetLastError() }.0;
! 400         return Err(Error::Security(SecurityError::InternalError(format!(
! 401             "MSQAAcquireToken failed (Windows error {code})"
! 402         ))));
! 403     }
  404     // Deletes the request when this function returns, on every path.
! 405     let _request = RequestGuard { api, request };
  406 
! 407     let status = unsafe { (api.get_request_status)(request) };
! 408     let status = if status == MSQA_INTERACTION_REQUIRED {
! 409         debug!("interactive: OneAuth requires sign-in, opening the host window");
! 410         run_interactive_ui(api, request, window_title, ui_thread_id)?
  411     } else {
! 412         debug!(status, "interactive: OneAuth answered without a prompt");
! 413         status
  414     };
  415 
! 416     if status != MSQA_SUCCESS {
! 417         return Err(describe_failure(api, request, status));
! 418     }
  419 
  420     // The account is now signed in; let later connections reuse the cached
  421     // account instead of prompting again.
! 422     unsafe { (api.set_option)(context.handle.0, MSQA_OPTION_FORCE_PROMPT, 0) };
! 423     read_access_token(api, request)
! 424 }
  425 
  426 /// Runs OneAuth's sign-in window to completion on a dedicated STA thread and
  427 /// returns the resulting request status.
  428 ///

---

  429 /// The window is created and pumped on the same thread because OneAuth posts
  430 /// its completion callback to that thread's message queue; the callback turns
  431 /// it into `WM_QUIT`, which ends the pump. Mirrors `MSQAThread`
  432 /// (`SNI_FedAuth.cpp:417-511`).
! 433 fn run_interactive_ui(
! 434     api: &'static MsqaApi,
! 435     request: HMsqaRequest,
! 436     window_title: &str,
! 437     ui_thread_id: &Arc<UiThreadId>,
! 438 ) -> TdsResult<i32> {
! 439     let moved = RequestHandle(request);
! 440     let title: Vec<u16> = window_title
! 441         .encode_utf16()
! 442         .chain(std::iter::once(0))
! 443         .collect();
! 444     let ui_thread_id = Arc::clone(ui_thread_id);
  445 
! 446     let worker = std::thread::Builder::new()
! 447         .name("mssql-odbc-interactive-auth".to_string())
! 448         .spawn(move || {
! 449             let moved = moved;
! 450             unsafe { pump_sign_in_window(api, moved.0, &title, &ui_thread_id) }
! 451         });
  452 
  453     // The caller holds the request alive across this whole function, and it
  454     // owns the acquire lock, so reading the status back here is safe even when
  455     // the UI thread never started or died.
! 456     let worker = match worker {
! 457         Ok(worker) => worker,
! 458         Err(e) => {
! 459             error!(error = %e, "interactive: could not start the sign-in UI thread");
! 460             return Ok(unsafe { (api.get_request_status)(request) });
  461         }
  462     };
  463 
! 464     match worker.join() {
! 465         Ok(status) => status,
  466         Err(_) => {
! 467             error!("interactive: the sign-in UI thread panicked");
! 468             Ok(unsafe { (api.get_request_status)(request) })
  469         }
  470     }
! 471 }
  472 
  473 /// Initializes an STA, hands the request to OneAuth's window, and pumps
  474 /// messages until the completion callback posts `WM_QUIT`.
  475 ///

---

  476 /// # Safety
  477 ///
  478 /// `request` must be a live `HMSQAREQUEST` that no other thread is using for
  479 /// the duration of this call.
! 480 unsafe fn pump_sign_in_window(
! 481     api: &'static MsqaApi,
! 482     request: HMsqaRequest,
! 483     title: &[u16],
! 484     ui_thread_id: &UiThreadId,
! 485 ) -> TdsResult<i32> {
  486     // OneAuth's window is a COM single-threaded apartment object.
! 487     let com = unsafe { CoInitializeEx(None, COINIT_APARTMENTTHREADED) };
! 488     if com.is_err() {
! 489         error!(hresult = com.0, "interactive: CoInitializeEx failed");
  490         // Not a request status: OneAuth never saw this request, so it has no
  491         // description for it. Returning the HRESULT here would send it through
  492         // `describe_failure`, which would query `MSQAGetErrorDescription` about
  493         // a request that never failed and render the HRESULT as a OneAuth

---

  491         // description for it. Returning the HRESULT here would send it through
  492         // `describe_failure`, which would query `MSQAGetErrorDescription` about
  493         // a request that never failed and render the HRESULT as a OneAuth
  494         // status code.
! 495         return Err(Error::Security(SecurityError::InternalError(format!(
! 496             "the interactive sign-in thread could not initialize COM (HRESULT {:#010x})",
! 497             com.0
! 498         ))));
! 499     }
  500 
! 501     let thread_id = unsafe { GetCurrentThreadId() };
! 502     ui_thread_id.set(thread_id);
  503 
! 504     let created = unsafe {
! 505         (api.ui_create_host_window)(
! 506             request,
! 507             ui_complete,
! 508             std::ptr::from_ref(ui_thread_id).cast_mut().cast(),
! 509             GetDesktopWindow(),
! 510             std::ptr::null(),
! 511             title.as_ptr(),
! 512             0,
! 513             0,
! 514             std::ptr::null_mut(),
! 515         )
  516     };
  517 
! 518     if created == MSQA_SUCCESS {
  519         // The DSN test dialog can still hold the mouse; releasing it lets the
  520         // user interact with the sign-in window (`SNI_FedAuth.cpp:470`).
! 521         let _ = unsafe { ReleaseCapture() };
  522 
! 523         let mut message = MSG::default();
  524         // GetMessageW returns >0 for a message, 0 for WM_QUIT, and -1 on error.
  525         // `BOOL::as_bool()` is `!= 0`, so it would take the error for a message
  526         // and dispatch an uninitialized MSG, spinning here forever while the
  527         // caller blocks on this thread's join. msodbcsql tests `> 0`

---

  526         // and dispatch an uninitialized MSG, spinning here forever while the
  527         // caller blocks on this thread's join. msodbcsql tests `> 0`
  528         // (`SNI_FedAuth.cpp:474`).
  529         loop {
! 530             let pumped = unsafe { GetMessageW(&mut message, None, 0, 0) }.0;
! 531             if pumped < 0 {
! 532                 let code = unsafe { GetLastError() }.0;
! 533                 error!(
  534                     windows_error = code,
  535                     "interactive: GetMessageW failed, ending the message pump"
  536                 );
! 537                 break;
! 538             }
! 539             if pumped == 0 {
! 540                 break;
! 541             }
! 542             let _ = unsafe { TranslateMessage(&message) };
! 543             unsafe { DispatchMessageW(&message) };
  544         }
  545     } else {
! 546         let code = unsafe { GetLastError() }.0;
! 547         error!(
  548             hresult = created,
  549             windows_error = code,
  550             "interactive: MSQAUICreateHostWindow failed"
  551         );

---

  550             "interactive: MSQAUICreateHostWindow failed"
  551         );
  552     }
  553 
! 554     ui_thread_id.clear();
! 555     let status = unsafe { (api.get_request_status)(request) };
! 556     unsafe { CoUninitialize() };
! 557     Ok(status)
! 558 }
  559 
  560 /// OneAuth's completion callback. Ends the message pump for the window whose
  561 /// [`UiThreadId`] was passed as `data` when the window was created.
  562 ///

---

  571 ///
  572 /// Called by OneAuth with the `callback_data` supplied to
  573 /// `MSQAUICreateHostWindow`, which is always a `&UiThreadId` borrowed from an
  574 /// `Arc` the caller keeps alive past `MSQADeleteRequest`.
! 575 unsafe extern "system" fn ui_complete(_request: HMsqaRequest, data: *mut c_void) {
! 576     let ui_thread_id = unsafe { &*data.cast::<UiThreadId>() };
! 577     post_quit(ui_thread_id, "OneAuth signalled completion");
! 578 }
  579 
  580 /// Reads the acquired token with the two-pass length-then-buffer protocol
  581 /// `MSQAGetAccessToken` uses.
  582 ///

---

  584 /// NUL, which is why the second call allocates `length + 1` while still passing
  585 /// `length` as the buffer size, and why truncating to `length` yields the token
  586 /// alone. Were it ever to include the terminator, the truncation would leave an
  587 /// embedded NUL inside the bearer token and the server would reject the login.
! 588 fn read_access_token(api: &'static MsqaApi, request: HMsqaRequest) -> TdsResult<String> {
! 589     let mut length: u32 = 0;
! 590     unsafe { (api.get_access_token)(request, std::ptr::null_mut(), &mut length) };
! 591     if length == 0 {
! 592         return Err(Error::ProtocolError(
! 593             "mssql-auth reported a successful sign-in but returned no access token".to_string(),
! 594         ));
! 595     }
  596 
! 597     let mut buffer = vec![0u16; length as usize + 1];
! 598     let status = unsafe { (api.get_access_token)(request, buffer.as_mut_ptr(), &mut length) };
! 599     if status != MSQA_SUCCESS {
! 600         return Err(Error::Security(SecurityError::InternalError(format!(
! 601             "MSQAGetAccessToken failed (status {status})"
! 602         ))));
! 603     }
  604 
! 605     buffer.truncate(length as usize);
! 606     String::from_utf16(&buffer).map_err(|_| {
! 607         Error::ProtocolError("mssql-auth returned a malformed access token".to_string())
! 608     })
! 609 }
  610 
  611 /// Turns a failed request into an error, preserving OneAuth's own description
  612 /// and preserving the transient/permanent distinction the connection retry
  613 /// logic depends on.
! 614 fn describe_failure(api: &'static MsqaApi, request: HMsqaRequest, status: i32) -> Error {
! 615     let mut length: u32 = 0;
! 616     let mut packed: i64 = 0;
! 617     unsafe { (api.get_error_description)(request, std::ptr::null_mut(), &mut length, &mut packed) };
  618 
! 619     let mut description = String::new();
! 620     if length > 0 {
! 621         let mut buffer = vec![0u16; length as usize + 1];
! 622         let read = unsafe {
! 623             (api.get_error_description)(request, buffer.as_mut_ptr(), &mut length, &mut packed)
  624         };
! 625         if read == MSQA_SUCCESS {
! 626             buffer.truncate(length as usize);
! 627             description = String::from_utf16_lossy(&buffer);
! 628         }
! 629     }
! 630     if description.is_empty() {
! 631         description = format!("sign-in failed with status {status}");
! 632     }
  633 
  634     // OneAuth packs its `Status` into the high byte and the underlying error
  635     // into the remainder (`SNI_FedAuth.cpp:726-759`).
! 636     let one_auth_status = ((packed >> MSQA_STATUS_SHIFT) & 0xFF) as u8;
  637 
! 638     if is_transient(one_auth_status) {
! 639         warn!(
  640             status = one_auth_status,
  641             "interactive: transient failure acquiring a token"
  642         );
  643         // Transient faults stay `ConnectionError` so the provider may retry.
! 644         return Error::ConnectionError(format!("Entra interactive sign-in failed: {description}"));
! 645     }
  646 
! 647     Error::Security(SecurityError::AuthenticationDenied(format!(
! 648         "Entra interactive sign-in failed: {description}"
! 649     )))
! 650 }
  651 
  652 /// Mirrors `IsTransientError` (`SNI_FedAuth.cpp:301-306`).
  653 fn is_transient(status: u8) -> bool {
  654     matches!(

---

mssql-odbc/src/handles/dbc.rs

  90             .field(
  91                 "access_token",
  92                 &self.access_token.as_ref().map(|_| "<REDACTED>"),
  93             )
! 94             .field("login_timeout", &self.login_timeout)
  95             .finish()
  96     }
  97 }

---

mssql-tds/src/connection_provider/tds_connection_provider.rs

  147             // (e.g. interactive sign-in).
  148             let login_timeout = context.login_timeout.unwrap_or(context.connect_timeout);
  149             let deadline = match login_timeout {
  150                 1.. => Some(Instant::now() + Duration::from_secs(login_timeout.into())),
! 151                 _ => None,
  152             };
  153 
  154             // Try Shared Memory before SSRP for local named instances (Windows only).
  155             // SM doesn't need instance resolution — it uses the name directly.

---

  195                         // or the login was rejected — would recur on every transport,
  196                         // and falling through would relaunch the interactive browser.
  197                         // Surface it immediately.
  198                         if !err.is_transient_connect_error() {
! 199                             debug!(
  200                                 "Shared Memory failed permanently ({}), not falling through",
  201                                 err
  202                             );
! 203                             return Err(err);
  204                         }
  205                         debug!("Shared Memory failed ({}), falling through to SSRP", err);
  206                     }
  207                 }

---

---

🔗 Quick Links

View Azure DevOps Build · Coverage Report

[Vahid (Vahid-b)]

Pushed 816589d0: implemented the separate SQL_ATTR_LOGIN_TIMEOUT login timeout (full msodbcsql parity), replacing the earlier "enlarge connect_timeout to 330s" workaround.

mssql-tds core (additive, back-compatible):

• ClientContext gains login_timeout: Option<u32>. The outer login deadline now uses login_timeout.unwrap_or(connect_timeout), so callers that only set connect_timeout are byte-for-byte unchanged. connect_timeout remains the per-TCP-connect cap.

mssql-odbc:

• SQL_ATTR_LOGIN_TIMEOUT is now honored (stored on the DBC, applied to login_timeout at connect; 0 → wait indefinitely) instead of being a silent no-op.
• Interactive installs the 330s login-timeout default only when the app hasn't set SQL_ATTR_LOGIN_TIMEOUT, and leaves connect_timeout at its default — so an unreachable server fails fast on the per-connect cap while the browser/MFA round-trip still fits.

Net effect: the black-hole-server concern from the earlier review is resolved properly rather than papered over.

Validation: cargo bfmt + workspace cargo bclippy -D warnings clean; cargo test -p mssql-odbc --lib → 346 pass; changed mssql-tds modules (client_context, connection_provider) pass. Full nextest/coverage in CI.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

mssql-odbc/src/auth/interactive.rs:315

• READ_TIMEOUT is restarted for every read, so a local client that sends one byte every few seconds can monopolize this serial handler until the full redirect timeout and prevent the real browser callback from being processed. Apply one timeout to the entire request-line read (or track an absolute deadline), rather than timing each individual read.

        let n = match tokio::time::timeout(READ_TIMEOUT, stream.read(&mut chunk)).await {
            Ok(Ok(n)) if n > 0 => n,
            _ => break, // EOF, read error, or timeout
        };

mssql-odbc/src/auth/interactive.rs:435

• The token response is aggregated without any size limit. Because this endpoint is derived from server-provided FEDAUTHINFO, a malicious or malfunctioning HTTPS authority can stream an arbitrarily large body and exhaust the host application's memory before the outer login timeout fires. Read chunks with a small explicit cap appropriate for an OAuth JSON response and reject responses that exceed it.

    let status = response.status();
    let bytes = response
        .bytes()
        .await
        .map_err(|e| Error::ConnectionError(format!("failed to read token response: {e}")))?;

mssql-odbc/src/auth/interactive.rs:422

• The client uses reqwest's default redirect policy, which follows cross-origin and HTTPS-to-HTTP redirects. A 307/308 response from the token endpoint would therefore resend the authorization code and PKCE verifier to the redirect target, bypassing the module's HTTPS-only endpoint check. OAuth token endpoints do not need redirects here; disable them (or validate every redirect target before following it).

    let client = reqwest::Client::builder()
        .build()
        .map_err(|e| Error::ConnectionError(format!("failed to build HTTP client: {e}")))?;

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

mssql-tds/src/connection_provider/tds_connection_provider.rs:150

• This deadline still does not bound every phase as claimed. Both SSRP calls at lines 223/227 await their independent ssrp_timeout_ms, and LocalDB resolution at line 266 performs a blocking LocalDBStartInstance call without consulting deadline; for example, login_timeout = 1 with ssrp_timeout_ms = 10_000 can run for roughly ten seconds. Apply the remaining login budget to these resolution phases too, and add a regression test with distinct login and SSRP timeouts.

            let login_timeout = context.login_timeout.unwrap_or(context.connect_timeout);
            let deadline = match login_timeout {
                1.. => Some(Instant::now() + Duration::from_secs(login_timeout.into())),
                _ => None,
            };

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

mssql-tds/src/connection_provider/tds_connection_provider.rs:150

• This only calculates a deadline; it does not bound the SSRP and LocalDB awaits below (lines 223/227 and 266). For example, an app-set 1-second login timeout can still wait for a longer SSRP timeout before the retry loop ever checks deadline, so the new overall-login contract is not enforced. Wrap those resolution phases in the remaining budget, or apply the timeout around the whole action-chain future.

            let login_timeout = context.login_timeout.unwrap_or(context.connect_timeout);
            let deadline = match login_timeout {
                1.. => Some(Instant::now() + Duration::from_secs(login_timeout.into())),
                _ => None,
            };

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

mssql-tds/src/connection_provider/tds_connection_provider.rs:142

• This deadline does not actually bound every phase named here. The SSRP calls at lines 223/227 use only ssrp_timeout_ms, and LocalDB resolution at line 266 is also awaited without the remaining login budget. For example, a caller can set login_timeout = 1 and ssrp_timeout_ms = 30, yet named-instance resolution can run for 30 seconds before the deadline is checked. Wrap these resolution phases in the remaining deadline (and move blocking LocalDB work off the runtime thread) so login_timeout is truly an overall cap.

            // Compute the overall login deadline up front so it bounds every
            // phase of login — the shared-memory shortcut below, SSRP/LocalDB
            // resolution, and the connect/retry loop — not just the final
            // transport attempts. `login_timeout` falls back to `connect_timeout`
            // for callers that only set the historical single knob; `0` means

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

mssql-odbc/src/auth/interactive.rs:565

• A successful HTTP status with malformed JSON or an empty token is converted to ConnectionError, which is_transient_connect_error retries. That retry restarts the entire connection and opens a second browser even though the authority already returned a definitive invalid response. Classify response-validation failures as a non-transient protocol error.

    let token = serde_json::from_str::<TokenResponse>(body)
        .map(|token| token.access_token)
        .map_err(|e| Error::ConnectionError(format!("failed to parse token response: {e}")))?;
    if token.is_empty() {
        return Err(Error::ConnectionError(
            "token response contained an empty access token".into(),
        ));

[Vahid (Vahid-b)]

Heads-up before you re-review: this PR has been redesigned, so a fresh read is needed rather than a diff against your last review.

The loopback + PKCE + system-browser flow is gone. Interactive sign-in now delegates to mssql-auth.dll (OneAuth) — the same library msodbcsql loads. auth/interactive.rs is rewritten, auth/msqa.rs is new, and base64/getrandom/sha2/serde/serde_json are dropped.

Why: we present msodbcsql's own client id, so the two drivers have to behave identically to callers and to the service. Hand-rolling our own OAuth flow behind that client id was the wrong call — same identity, different behaviour.

What that means for your earlier comments: the threads about PKCE, state validation, redirect classification, browser launching and child-process reaping no longer refer to any code in this PR. Nothing was dismissed — the code they applied to was deleted.

Platform behaviour: interactive is Windows-only, matching msodbcsql (SNI_FedAuth is excluded from the Unix SRCLIST and the DLL is loaded via LoadLibraryExA with no dlopen counterpart). On Linux/macOS the keyword falls through to ActiveDirectoryIntegrated, which is what msodbcsql itself does at Parse.cpp:3597-3660. I prototyped an explicit "not supported on this platform" error and withdrew it — refusing a keyword msodbcsql accepts would be the one place the drivers visibly disagree.

Also still in scope: SQL_ATTR_LOGIN_TIMEOUT plus a real SQLGetConnectAttrW.

Verified live against Azure SQL, not just unit tests: sign-in completes, a second connection reuses the cached context without prompting (0.1s vs 19.4s), and cancelling fails exactly once without reopening the window.

One open question worth your input: mssql-auth.dll is not redistributed by this driver. Interactive works where msodbcsql 18 has already put it in System32. Packaging needs a decision before GA — happy to take that to a separate thread.

Details and parity citations are in the updated PR description.

[Copilot]

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Suppressed comments (6)

mssql-odbc/src/auth/msqa.rs:350

• This also recovers a poisoned mutex and proceeds with a context whose acquisition state may be inconsistent. Return an error on poison rather than using the inner value, as required for mutexes in the ODBC driver.

    let _serialized = context
        .acquire_lock
        .lock()
        .unwrap_or_else(|poisoned| poisoned.into_inner());

mssql-odbc/src/auth/msqa.rs:202

• This non-test expect violates the ODBC shared-library's panic-free requirement. Propagate the construction failure through the cached Result rather than introducing a panic path, even though the current constant is valid.

        let name = CString::new(MSQA_LIBRARY).expect("library name has no interior NUL");

mssql-odbc/src/auth/msqa.rs:612

• A zero id does not necessarily mean there is nothing left to cancel: the spawn_blocking task may still be queued or waiting on acquire_lock and has not called set yet. If the login deadline expires in that window, this returns, then the detached task can later open a sign-in window with no caller remaining to close it. Record cancellation independently of the thread id and have registration/UI startup observe that flag before creating the window.

pub(super) fn cancel_ui(ui_thread_id: &UiThreadId) {
    let thread_id = ui_thread_id.get();
    if thread_id == 0 {
        return;

mssql-odbc/src/auth/msqa.rs:282

• Recovering a poisoned mutex with into_inner() is unsafe for this in-process driver because the cache may have been left inconsistent by the panic. Follow the crate's poison policy and return an authentication error instead of continuing with potentially corrupted state.

This issue also appears on line 347 of the same file.

    let mut cache = cache
        .lock()
        .unwrap_or_else(|poisoned| poisoned.into_inner());

mssql-odbc/src/auth/interactive.rs:150

• mem::forget leaks the guard's Arc on every completed token acquisition. Disarm the guard with explicit state (for example, an Option/armed flag checked by Drop) so successful calls release the allocation without posting cancellation.

    fn disarm(self) {
        std::mem::forget(self);

mssql-odbc/src/auth/msqa.rs:278

• The PR description says context-cache keying and error-description reads are unit-tested, but this module's tests never call get_or_create_context or describe_failure; they only test status constants, the atomic id, NUL rejection, and bit extraction. Add injectable/fake MSQA API tests that verify reuse/separation by (login_hint, sts_url) and both passes of error-description retrieval, or update the stated coverage.

    let key = (login_hint.to_string(), sts_url.to_string());

[Vahid (Vahid-b)]

Review thread triage

I went back through all 23 open threads and validated each one against the code as it stands now rather than as it stood when the comment was written. Two pushes since: 1f0a7b6 and 0f08e9c.

One was a live bug and is fixed (0f08e9c) - msqa.rs message pump. GetMessageW returns -1 on error, and BOOL::as_bool() is a plain != 0, so the error return was read as "message available". Because run_interactive_ui blocks on the thread join, a persistent error would have hung the connection and leaked the STA thread. msodbcsql tests > 0 (SNI_FedAuth.cpp:474); we now do too. Thanks Copilot - real find.

One more parity gap I found while re-reading, fixed (1f0a7b6) - SQL_ATTR_LOGIN_TIMEOUT was not clamped. msodbcsql caps at MAX_QUERY_TIMEOUT (0xfffe) and posts 01S02 "Login timeout changed" (sqlcmisc.cpp:1735). We accepted anything and narrowed with a raw as u32, so a request for 2^32 seconds wrapped to 0, which the connect path reads as wait indefinitely - the opposite of the caller's intent. Clamping at pointer width before narrowing makes the wrap unreachable. Three tests added.

14 were obsolete, all describing the browser/loopback/PKCE flow that the MSQA pivot deleted. Verified by symbol, not assumption - xdg-open, /usr/bin/open, TcpListener, reqwest, code_verifier, READ_TIMEOUT and the authorization-URL logging are all gone from mssql-odbc. The OnceCell flagged in interactive.rs is gone as well; the one left in entra.rs caches the credential, not a token, so the staleness concern does not carry over.

6 were fixed in earlier rounds and simply never marked resolved - the get_connect_attr.rs cluster (layering now ffi_entry! -> unsafe shim -> safe core, write_if_some, full entry logging, canonical ERR_INVALID_NULL_POINTER, HYC00 for unsupported attributes), the login-deadline placement, and the shared-memory Err arm now surfacing non-transient failures instead of falling through to SSRP and re-prompting.

1 was valid but inert - ClientContext field addition. Answered in-thread: the crate is unpublished and pre-1.0, and no sibling crate constructs it with a struct literal, so nothing can break. #[non_exhaustive] is worth doing, but as its own API change.

Those 22 are now resolved. I left Shiwani Gupta (@shiwanigupta0809)'s client-id question open since it is hers to close - short version: the id was wrong when you asked and is now taken verbatim from Parse.cpp:3608, with a test pinning it.

[David Engel (David-Engel)]

Review summary

Adds Authentication=ActiveDirectoryInteractive to mssql-odbc by delegating to mssql-auth.dll (OneAuth/MSQA) on Windows, splits connect_timeout into a per-TCP-connect cap and a new overall login_timeout, and implements a real SQLGetConnectAttrW for SQL_ATTR_LOGIN_TIMEOUT.

The redesign away from a hand-rolled PKCE flow is the right call, and the FFI layer is careful: RAII request guard, dedicated STA thread, the GetMessageW > 0 fix, embedded-NUL rejection, and traceable msodbcsql references throughout. cargo test -p mssql-odbc --lib passes locally on Linux (440 tests).

Findings are inline. Two I'd like resolved before merge:

1. WM_QUIT can be posted to a recycled thread id in cancel_ui — the load-then-post is unsynchronized, and in an ODBC driver hosted inside an arbitrary application this can tear down the host app's message loop.
2. The off-Windows diagnostic names a method the caller never asked for — a Linux user who writes Authentication=ActiveDirectoryInteractive gets "Authentication method ActiveDirectoryIntegrated is not yet supported", which is undiagnosable.

The rest are suggestions and nits.

Tests

Coverage of the pure logic is good: clamping, 0 to infinite, app-set precedence, transient classification, cache-key round-trip, NUL rejection. The gap is that roughly 690 lines of msqa.rs — the pump, the cancel path, the token and error reads — are exercised only by the manual live run, and both blocking findings live in exactly that untested region. Given the 85% bar, consider extracting the cancel/thread-id lifecycle behind a small trait so the race is testable without the DLL.

[Vahid (Vahid-b)]

Re-queued validation (build 164519). The previous run failed only on mssql-py-core's test_tracing_end_to_end, a known intermittent flake: the test waits on tracing_appender::non_blocking's background writer with a fixed thread::sleep(100ms), so on a loaded agent the log file is only partially flushed when it is read. Same test failed the same way on PR #141 (build 162171) and PR #143 (build 163026), and the failing assertion moves between the first and second message across runs. Nothing in this PR touches mssql-py-core.

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).

[David Engel (David-Engel)]

Re-review

All eleven findings from the previous pass are addressed, and several went further than what I asked for:

• The thread-id fix also re-routes ui_complete through the guarded path, so a late OneAuth callback can't post at a recycled thread either — not just the deadline canceller.
• The HY011 question was answered better than I posed it: the guard belongs on SQL_ATTR_PACKET_SIZE (fixed by LOGIN7) and not on SQL_ATTR_LOGIN_TIMEOUT, with the msodbcsql citation showing it stores that one unconditionally on a reusable handle.
• The async sign_in_lock correctly keeps the guard inside the blocking task, so a dropped future can't release it while the window is still tearing down. The current-thread-runtime tests are a neat way to prove the waiter yields rather than parks.

cargo test -p mssql-odbc --lib passes (446) and cargo clippy -p mssql-odbc -p mssql-tds --all-targets is clean on Linux.

Two nits inline, neither merge-blocking.

Where I'd still spend manual attention

Everything remaining is Windows-only FFI that CI cannot reach:

1. ui_complete's pointer lifetime. The callback now dereferences data as &UiThreadId. The ordering in the code is right — MSQADeleteRequest runs inside acquire_token before the closure's Arc drops — but the safety argument rests on OneAuth never invoking the callback after the request is deleted, which is an assumption about a closed-source DLL. The fix traded a narrow stray-WM_QUIT window for a use-after-free invariant, so it's worth confirming the MSQA contract actually guarantees it.
2. Live re-run of the deadline-expiry-with-window-open path, since post_quit now holds a lock across PostThreadMessageW while the pump may be inside DispatchMessageW. PostThreadMessageW is documented as non-blocking so this should be fine, but it is exactly the path the change touches and no test can reach it.
3. A DM smoke test for the new SQL_ATTR_PACKET_SIZE HY011, which is new behaviour for every connection rather than just interactive ones.

[Saurabh Singh (saurabh500)]

Make sure to merge this branch with the latest origin/main to get the benefits of coverage reporting.

[Vahid (Vahid-b)]

Saurabh Singh (@saurabh500) Done — merged origin/main into the branch in ae97302. Clean merge, no conflicts; the branch was two commits behind and has picked up #167, so the Windows ODBC C++ e2e coverage now flows into this PR's diff coverage.

Head is now 40c0f70.