From d6a94750876cc6fac5838e04270072ff08e297bf Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Wed, 12 Aug 2026 10:27:04 -0400 Subject: [PATCH 1/2] Give the WAM broker a parent window handle so Entra MFA works (#2184) Current Microsoft.Data.SqlClient (7.0.2) routes ActiveDirectoryInteractive through the Windows WAM broker, which requires the application to supply the HWND that will own its account picker. Lite set the authentication mode but never registered an auth provider, so every Entra MFA connection failed with 0xwindow_handle_required instead of prompting. Register a process-wide provider at startup (covers the Add/Edit dialog's Test Connection and every collector loop), resolving the owning window per prompt - the active window first, so the picker lands in front of the dialog rather than behind it - marshaled through the dispatcher because MSAL calls from whatever thread the connection open runs on, and WPF throws on off-thread Window property reads. The blocking Invoke is safe: no UI-thread path blocks on a SQL connection open. Direct port of the Studio fix (PerformanceStudio#426), verified there against a real Entra-MFA tenant by this issue's reporter, including the silent-SSO case (broker satisfies MFA from the Windows session and no picker appears - that is WAM working). Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 3 + Lite.Tests/EntraInteractiveAuthTests.cs | 48 +++++++++++++ Lite/App.xaml.cs | 67 +++++++++++++++++++ Lite/Services/EntraInteractiveAuth.cs | 89 +++++++++++++++++++++++++ 4 files changed, 207 insertions(+) create mode 100644 Lite.Tests/EntraInteractiveAuthTests.cs create mode 100644 Lite/Services/EntraInteractiveAuth.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 45ba6a7738..79ce51172b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Microsoft Entra MFA can actually connect - Lite** ([#2184], reported by @joshdbe) - current Microsoft.Data.SqlClient routes interactive Entra auth through the Windows WAM broker, and WAM requires the application to hand it the window that will own its account picker. Lite set `ActiveDirectoryInteractive` on the connection string but never registered an auth provider, so every Entra MFA connection died with `0xwindow_handle_required` instead of prompting - on every machine, not some particular tenant, and old builds only worked because they predate WAM being SqlClient's default. Lite now registers a process-wide provider at startup that resolves the owning window per prompt (the Add/Edit Server dialog in front, not the main window behind it), marshaled to the UI thread because MSAL asks from whatever thread the connection open happens to run on - collector loops included. One behavior worth knowing on sight: when the Windows session already satisfies MFA, the broker succeeds silently and no picker appears at all - that is WAM working, not the prompt failing. Same seam as Performance Studio's fix (PerformanceStudio#426), which the reporter verified against a real Entra-MFA tenant; the silent-SSO behavior above is exactly what his verification observed. + - **Dropped databases no longer leave Query Store collection state behind forever - both apps** ([#2188]) - Query Store collection writes one `collector_state` row per database and never deleted any of them for a database that was dropped or renamed: the #2164 plan-XML watermark (`planwm:`, Darling only) and the #2022/#2058 backfill worker's tail and gap markers (`done:` / `hole:`, BOTH apps - the worker deletes a hole when it services or expires it, but a dropped database can never do either). On a server with churny database lifecycles - dev/test, or multi-tenant provisioning - those accumulated with no pruning path, since `collector_state` is a keyed registry rather than a hypertable and has no retention policy to catch them. Both apps now retire them on the query_store cycle, from one shared list of which keys are database-keyed, so a future key cannot end up pruned on one app and orphaning on the other. The delete is keyed on `database_states` - the unfiltered `sys.databases` snapshot - and deliberately NOT on query_store's own enumeration, which screens out offline databases, AG secondaries, excluded databases and anything that failed its probe: pruning on absence from that list would delete LIVE state on exactly the servers that keep databases parked or excluded, and the only symptom would be a silent re-fetch. It also requires the snapshot to be NEWER than the row it judges, so a server whose database_states collection has stopped cannot have every database created since then pruned on repeat. A server with no snapshot at all prunes nothing rather than everything, which is what keeps an Azure SQL Database server (where `database_states` is not collected, [#2191]) and a server whose snapshots have aged out from being mass deletes. Cosmetic either way - an orphaned row is simply never read again - and a same-name recreate is bounded by the watermark's own one-day refresh horizon, not by this. - **A managed-Postgres bootstrap failure now says what went wrong instead of printing a Win32 number** ([#2186], from @jovon44's report in #2185) - a store that could not start reported `initdb failed (exit code -1073741515) for ... Output:` and nothing else. `-1073741515` is `0xC0000135`, `STATUS_DLL_NOT_FOUND`: Windows killed the process in the loader, before a line of its own code ran - which is also why `Output:` was blank and always would be, so the one field an operator reads was guaranteed empty exactly when the failure was a load failure. Their attention then went to the follow-on missing-credential message and `darling.json`, neither of which was the fault. Every bundled-binary failure (initdb, pg_ctl start/status/stop/reload, pg_upgrade and its `--check`, the post-upgrade analyze) now decodes a Windows status into its name, names the two causes that account for nearly all of them - the bundled MSVC runtime absent from `pg-runtime\pgsql\bin`, which means a partial or damaged extract rather than a missing prerequisite, or a service account that cannot read an install tree sitting under a user profile - and gives the two checks that tell them apart, including the caveat that these tools re-execute themselves under a restricted token that drops Administrators, so running the binary by hand succeeding does not clear the permissions theory. An empty `Output:` now says it is empty **because** the process was killed before it could write, rather than looking like data that failed to arrive. Two messages also stopped blaming the wrong thing: `pg_ctl status` no longer calls the data directory unusable when the verdict came from Windows rather than from pg_ctl (that phrasing pointed at deleting a healthy store to fix a missing DLL), and `pg_upgrade --check` no longer reports clusters as incompatible when pg_upgrade never ran to form an opinion. The related [#1738] refusal now quotes and decodes the probe exit code it always had, instead of asserting that the binaries did not run without saying how it knew. - **A missing store credential no longer reads as a first run after a bootstrap has already failed** ([#2197], the other half of @jovon44's #2185) - when a managed bootstrap dies, the operator's last message is rarely the bootstrap error; it is whatever CLI verb they run next, and every one of those said `Start the PerformanceMonitor Darling service once so its first run initializes the store`. That is correct advice for a genuine first run and a dead end for the case that actually produces it in the field - the service HAS been started, its bootstrap failed, and starting it again fails the same way. In #2185 that is the message the reporter led with, and it is what sent them to `darling.json`, which was never the fault. The six copies of that sentence (five CLI verbs, plus the Viewer's managed-mode parse whose text the main window shows) are now one shared message that decides between the two from the STORE'S OWN FILES: an initialized cluster, a `pg.log`, or a credential file beside the data directory - in particular the store's own credential, which the service writes immediately before it runs initdb and which therefore survives the exact failure [#2186] decodes. With evidence it says this is not a first run, quotes what it found so the verdict is checkable, and points at `%ProgramData%\PerformanceMonitorDarling\logs\darling-service_yyyyMMdd.log` - where, since [#2186], a bundled Postgres tool that Windows killed explains itself in words rather than as a bare exit code. Without evidence the first-run advice is unchanged, and gains the one sentence it was missing for the operator who has already started it. What it deliberately never infers from is a machine-global signal such as the service log directory existing: that would tell somebody standing up a SECOND store on a working box that their bootstrap had failed, which is this same defect pointed somewhere new, and an empty data directory an operator pre-created is not evidence either. The four store-credential verbs also name the credential file's path now, which none of them did. @@ -2688,3 +2690,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#2203]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2203 [#2187]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2187 [#2189]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2189 +[#2184]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/2184 diff --git a/Lite.Tests/EntraInteractiveAuthTests.cs b/Lite.Tests/EntraInteractiveAuthTests.cs new file mode 100644 index 0000000000..b57014bb6c --- /dev/null +++ b/Lite.Tests/EntraInteractiveAuthTests.cs @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor Lite. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using PerformanceMonitorLite.Services; +using Xunit; + +namespace PerformanceMonitorLite.Tests; + +/// +/// #2184: interactive Entra auth needs a parent window handle because SqlClient routes it through the +/// WAM broker. These pin the wiring contracts that are verifiable without a tenant — argument +/// validation and the idempotent process-wide registration. Whether the picker actually authenticates +/// is a real-tenant step: the Studio twin of this seam (PerformanceStudio#426) was verified by this +/// issue's reporter against his Entra-MFA-on-Azure-VM environment. +/// +public class EntraInteractiveAuthTests +{ + [Fact] + public void Register_RejectsANullHandleProvider() + { + /* The whole point of the type is supplying a handle; accepting null would register a provider + that fails at prompt time instead of at wiring time, which is the harder bug to find. */ + Assert.Throws(() => EntraInteractiveAuth.Register(null!)); + } + + [Fact] + public void Register_IsProcessWideAndIdempotent() + { + /* SqlAuthenticationProvider.SetProvider is process-wide and a second registration would + silently replace the first, so "first one wins, later ones are no-ops" is the contract worth + pinning — the app registers at startup and nothing else should be able to swap the provider + out from under it. The reset makes registration order observable regardless of what ran + earlier in the test process. */ + EntraInteractiveAuth.ResetRegistrationForTests(); + + var first = EntraInteractiveAuth.Register(() => IntPtr.Zero); + var second = EntraInteractiveAuth.Register(() => new IntPtr(1234)); + + Assert.True(first, "the first registration after reset must install the provider"); + Assert.False(second, "a second Register must be a no-op rather than replacing the provider"); + } +} diff --git a/Lite/App.xaml.cs b/Lite/App.xaml.cs index 655668acad..f7d1aa8c59 100644 --- a/Lite/App.xaml.cs +++ b/Lite/App.xaml.cs @@ -16,6 +16,7 @@ using System.Threading; using System.Threading.Tasks; using System.Windows; +using System.Windows.Interop; using PerformanceMonitor.Notifications; using System.Windows.Threading; using PerformanceMonitorLite.Services; @@ -459,11 +460,77 @@ land in the file a few statements later. */ DispatcherUnhandledException += OnDispatcherUnhandledException; TaskScheduler.UnobservedTaskException += OnUnobservedTaskException; + /* Entra MFA needs a parent window handle for the WAM broker, or interactive auth fails with + 0xwindow_handle_required instead of prompting (#2184). Registered once, before any window + exists, because SqlAuthenticationProvider installs process-wide: the Add/Edit dialog's Test + Connection and every collector connection are covered without per-site wiring. The handle + itself is resolved lazily per prompt, so registering this early is safe. */ + Services.EntraInteractiveAuth.Register(ActiveWindowHandle); + // Create and show main window (StartupUri removed for Velopack custom Main) _mainWindow = new MainWindow(); _mainWindow.Show(); } + /// + /// The window that should own an Entra MFA prompt, resolved at the moment MSAL asks (#2184). + /// + /// Prefers whichever window is currently active over the main window, because a connection is + /// usually triggered from the Add/Edit Server dialog — parenting the account picker to the main + /// window behind it would let the picker appear behind the dialog the user is looking at. Falls back + /// to the main window, then to , which MSAL treats the same as no handle: + /// the prompt fails rather than the app crashing, which is the right way round for an auth path. + /// + /// Resolved per call rather than captured once: a window's HWND does not exist until the + /// window has been sourced, and the right parent is whichever window is in front now, not the one + /// that existed at startup. + /// + /// Marshaled to the UI thread: MSAL invokes this from whatever thread SqlClient's token + /// acquisition runs on — collector worker threads included — and WPF enforces dispatcher affinity + /// on properties, so an off-thread read would throw rather than merely race. + /// The blocking Invoke is safe here because no UI-thread path blocks on a SQL connection open + /// (opens are async throughout; the UI stays pumping). If the dispatcher cannot deliver anyway + /// (shutdown timing), Zero degrades to MSAL's normal no-handle failure instead of throwing from + /// inside the auth callback. + /// + private static IntPtr ActiveWindowHandle() + { + try + { + var dispatcher = Current?.Dispatcher; + if (dispatcher is null) + return IntPtr.Zero; + + return dispatcher.CheckAccess() + ? ActiveWindowHandleOnUIThread() + : dispatcher.Invoke(ActiveWindowHandleOnUIThread); + } + catch + { + return IntPtr.Zero; + } + } + + private static IntPtr ActiveWindowHandleOnUIThread() + { + var app = Current; + if (app is null) + return IntPtr.Zero; + + Window? active = null; + foreach (Window window in app.Windows) + { + if (window.IsActive) + { + active = window; + break; + } + } + + var owner = active ?? app.MainWindow; + return owner is null ? IntPtr.Zero : new WindowInteropHelper(owner).Handle; + } + /// /// Invoked on 's background thread when a second launch asks us /// to surface the window. Marshals to the UI thread and restores via WPF's Show() path (#1050). diff --git a/Lite/Services/EntraInteractiveAuth.cs b/Lite/Services/EntraInteractiveAuth.cs new file mode 100644 index 0000000000..673d823318 --- /dev/null +++ b/Lite/Services/EntraInteractiveAuth.cs @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor Lite. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using Microsoft.Data.SqlClient; + +namespace PerformanceMonitorLite.Services; + +/// +/// Makes Authentication=ActiveDirectoryInteractive (Microsoft Entra MFA) work by giving MSAL a +/// parent window handle (#2184). +/// +/// Why this has to exist: current Microsoft.Data.SqlClient (7.0.2 here) routes interactive +/// Entra auth through the Windows WAM broker, and WAM requires the calling application to supply +/// the HWND that will own its account picker. An application that never supplies one does not get a +/// prompt — it gets 0xwindow_handle_required / "A window handle must be configured" and the +/// connection fails outright. Lite set the authentication mode in +/// but never registered a provider, so Entra +/// MFA was broken for every user on a current build, not for some particular tenant. Old builds worked +/// because they predate WAM being SqlClient's default and fell back to a browser. +/// +/// Registration is process-wide. installs +/// against the authentication METHOD, not a connection, so one call at startup covers every +/// SqlConnection the process opens — the Add/Edit dialog's Test Connection and every collector +/// loop — without threading a handle through call sites that could each forget it. +/// +/// Same seam as Performance Studio's fix +/// (PerformanceStudio#426), +/// which was verified against a real Entra-MFA tenant by the reporter of #2184; only the handle source +/// differs (WPF's WindowInteropHelper here vs Avalonia's platform handle there). +/// +public static class EntraInteractiveAuth +{ + private static readonly object Gate = new(); + private static bool _registered; + + /// + /// Registers the interactive-auth provider, resolving the parent window through + /// at the moment MSAL asks for it. + /// + /// The handle is fetched per prompt rather than captured once, deliberately: a window's HWND + /// does not exist until the window is sourced, and the right parent is whichever window is actually + /// in front when a connection fires — usually the Add/Edit Server dialog, not the main window + /// behind it. + /// + /// + /// Returns the owning window handle, or when no window is available. + /// Zero degrades to MSAL's normal no-handle failure rather than a crash. + /// + /// True if this call registered the provider; false if already registered. + public static bool Register(Func parentWindowHandleProvider) + { + ArgumentNullException.ThrowIfNull(parentWindowHandleProvider); + + lock (Gate) + { + if (_registered) + return false; + + var provider = new ActiveDirectoryAuthenticationProvider(); + + /* Func rather than Func is MSAL's shape — it takes an Android Activity on + mobile and an HWND on Windows. Boxing the IntPtr is the intended usage. */ + provider.SetParentActivityOrWindowFunc(() => parentWindowHandleProvider()); + + SqlAuthenticationProvider.SetProvider(SqlAuthenticationMethod.ActiveDirectoryInteractive, provider); + + _registered = true; + return true; + } + } + + /* Test-only escape hatch for the one-way registration flag. Registration is deliberately + irreversible in production — SqlAuthenticationProvider offers no unregister — so a test that + needs to observe registration order resets the flag rather than the provider. The provider may + stay registered with SqlClient; tests open no interactive connections, so that is inert. */ + internal static void ResetRegistrationForTests() + { + lock (Gate) + { + _registered = false; + } + } +} From ec039168287ff9623c2a3c32acafbaf07911c668 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Wed, 12 Aug 2026 10:43:41 -0400 Subject: [PATCH 2/2] Address review: log the handle-resolution fallback instead of failing silently Zero is exactly the input that reproduces 0xwindow_handle_required, so a throw inside ActiveWindowHandle must leave a trace in the log - a silent fallback would be the #2184 bug's own shape one layer down, as the review put it. Guarded like the file's other late-lifecycle log calls, because handle resolution can fire during shutdown after the logger is gone. Co-Authored-By: Claude Fable 5 --- Lite/App.xaml.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Lite/App.xaml.cs b/Lite/App.xaml.cs index f7d1aa8c59..512f0a1333 100644 --- a/Lite/App.xaml.cs +++ b/Lite/App.xaml.cs @@ -505,8 +505,13 @@ private static IntPtr ActiveWindowHandle() ? ActiveWindowHandleOnUIThread() : dispatcher.Invoke(ActiveWindowHandleOnUIThread); } - catch + catch (Exception ex) { + /* Zero reproduces the original #2184 symptom (0xwindow_handle_required), so a throw here + must leave a trace - a silent fallback would be this bug's own shape one layer down. The + log call is guarded because this can fire during shutdown, after the dispatcher and + logger are gone, and logging must never be the thing that breaks auth. */ + try { AppLogger.Warn("App", $"Entra parent-window handle resolution failed; WAM will see no handle: {ex.Message}"); } catch { /* nothing left to log to */ } return IntPtr.Zero; } }