Give the WAM broker a parent window handle so Entra MFA works (#2184)

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

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;
+
+/// <summary>
+/// #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.
+/// </summary>
+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<ArgumentNullException>(() => 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");
+    }
+}

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,82 @@ 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();
     }
 
+    /// <summary>
+    /// The window that should own an Entra MFA prompt, resolved at the moment MSAL asks (#2184).
+    ///
+    /// <para>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 <see cref="IntPtr.Zero"/>, 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.</para>
+    ///
+    /// <para>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.</para>
+    ///
+    /// <para>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 <see cref="Window"/> 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.</para>
+    /// </summary>
+    private static IntPtr ActiveWindowHandle()
+    {
+        try
+        {
+            var dispatcher = Current?.Dispatcher;
+            if (dispatcher is null)
+                return IntPtr.Zero;
+
+            return dispatcher.CheckAccess()
+                ? ActiveWindowHandleOnUIThread()
+                : dispatcher.Invoke(ActiveWindowHandleOnUIThread);
+        }
+        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;
+        }
+    }
+
+    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;
+    }
+
     /// <summary>
     /// Invoked on <see cref="SingleInstanceSignal"/>'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).

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;
+
+/// <summary>
+/// Makes <c>Authentication=ActiveDirectoryInteractive</c> (Microsoft Entra MFA) work by giving MSAL a
+/// parent window handle (#2184).
+///
+/// <para>Why this has to exist: current <c>Microsoft.Data.SqlClient</c> (7.0.2 here) routes interactive
+/// Entra auth through the Windows <b>WAM broker</b>, 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 <c>0xwindow_handle_required</c> / "A window handle must be configured" and the
+/// connection fails outright. Lite set the authentication mode in
+/// <see cref="Models.ServerConnection.ApplyAuthentication"/> 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.</para>
+///
+/// <para>Registration is process-wide. <see cref="SqlAuthenticationProvider.SetProvider"/> installs
+/// against the authentication METHOD, not a connection, so one call at startup covers every
+/// <c>SqlConnection</c> 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.</para>
+///
+/// <para>Same seam as Performance Studio's fix
+/// (<see href="https://github.com/erikdarlingdata/PerformanceStudio/pull/426">PerformanceStudio#426</see>),
+/// which was verified against a real Entra-MFA tenant by the reporter of #2184; only the handle source
+/// differs (WPF's <c>WindowInteropHelper</c> here vs Avalonia's platform handle there).</para>
+/// </summary>
+public static class EntraInteractiveAuth
+{
+    private static readonly object Gate = new();
+    private static bool _registered;
+
+    /// <summary>
+    /// Registers the interactive-auth provider, resolving the parent window through
+    /// <paramref name="parentWindowHandleProvider"/> at the moment MSAL asks for it.
+    ///
+    /// <para>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.</para>
+    /// </summary>
+    /// <param name="parentWindowHandleProvider">
+    /// Returns the owning window handle, or <see cref="IntPtr.Zero"/> when no window is available.
+    /// Zero degrades to MSAL's normal no-handle failure rather than a crash.
+    /// </param>
+    /// <returns>True if this call registered the provider; false if already registered.</returns>
+    public static bool Register(Func<IntPtr> parentWindowHandleProvider)
+    {
+        ArgumentNullException.ThrowIfNull(parentWindowHandleProvider);
+
+        lock (Gate)
+        {
+            if (_registered)
+                return false;
+
+            var provider = new ActiveDirectoryAuthenticationProvider();
+
+            /* Func<object> rather than Func<IntPtr> 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;
+        }
+    }
+}