diff --git a/docs/environment.md b/docs/environment.md index 424867a68..888e13adb 100644 --- a/docs/environment.md +++ b/docs/environment.md @@ -173,6 +173,40 @@ Defaults to enabled. --- +### GCM_MODAL_PARENTHWND + +Specify the parent window handle (`HWND`) for modal dialogs on Windows. + +> [!NOTE] +> This is intended for use by application developers who are calling Git and/or +> GCM from their application and wish to correctly parent authentication dialogs +> to an existing window. +> +> Do **not** set this manually. + +#### Example + +##### Windows + +```csharp +public static int Example(IntPtr myWindowHandle) +{ + using var process = new Process(); + process.StartInfo = new ProcessStartInfo("git.exe", "pull") + { + Environment = + { + ["GCM_MODAL_PARENTHWND"] = myWindowHandle.ToString("d") + } + }; + process.Start(); + process.WaitForExit(); + return process.ExitCode; +} +``` + +--- + ### GCM_PROVIDER Define the host provider to use when authenticating. diff --git a/src/Core.Tests/Authentication/Entra/MsalParentWindowAdapterTests.cs b/src/Core.Tests/Authentication/Entra/MsalParentWindowAdapterTests.cs new file mode 100644 index 000000000..831ae489f --- /dev/null +++ b/src/Core.Tests/Authentication/Entra/MsalParentWindowAdapterTests.cs @@ -0,0 +1,138 @@ +using System; +using System.Threading; +using GitCredentialManager.Authentication.Entra; +using Xunit; + +namespace GitCredentialManager.Tests.Authentication.Entra; + +public class MsalParentWindowAdapterTests +{ + [Theory] + [InlineData(42, false)] + [InlineData(42, true)] + [InlineData(-1, false)] + [InlineData(-1, true)] + public void GetWindow_ExplicitParent_TakesPrecedence(int parentValue, bool createIfMissing) + { + object parentWindow = new IntPtr(parentValue); + using var adapter = new MsalParentWindowAdapter(parentWindow, createIfMissing, + () => throw new InvalidOperationException("Console lookup should not be called."), + _ => throw new InvalidOperationException("Visibility should not be checked."), + _ => throw new InvalidOperationException("A progress window should not be created.")); + + object result = adapter.GetWindow(); + Assert.Same(parentWindow, result); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void GetWindow_VisibleConsoleRoot_PrecedesProgressWindow(bool createIfMissing) + { + var consoleRoot = new IntPtr(42); + bool visibilityChecked = false; + using var adapter = new MsalParentWindowAdapter(IntPtr.Zero, createIfMissing, + () => consoleRoot, + hwnd => + { + Assert.Equal(consoleRoot, hwnd); + visibilityChecked = true; + return true; + }, + _ => throw new InvalidOperationException("A progress window should not be created.")); + + IntPtr result = Assert.IsType(adapter.GetWindow()); + + Assert.Equal(consoleRoot, result); + Assert.True(visibilityChecked); + } + + [Theory] + [InlineData(0)] + [InlineData(42)] + public void GetWindow_NoVisibleConsoleRoot_CreatesProgressWhenRequired(int consoleRootValue) + { + var consoleRoot = new IntPtr(consoleRootValue); + var progressWindow = new IntPtr(84); + int visibilityChecks = 0; + var windowToken = CancellationToken.None; + using (var adapter = new MsalParentWindowAdapter(IntPtr.Zero, true, + () => consoleRoot, + hwnd => + { + Assert.Equal(consoleRoot, hwnd); + visibilityChecks++; + return false; + }, + ct => + { + windowToken = ct; + return progressWindow; + })) + { + IntPtr result = Assert.IsType(adapter.GetWindow()); + + Assert.Equal(progressWindow, result); + Assert.Equal(consoleRootValue == 0 ? 0 : 1, visibilityChecks); + Assert.True(windowToken.CanBeCanceled); + Assert.False(windowToken.IsCancellationRequested); + } + + Assert.True(windowToken.IsCancellationRequested); + } + + [Theory] + [InlineData(0)] + [InlineData(42)] + public void GetWindow_NoVisibleConsoleRoot_ReturnsNullWhenParentNotRequired(int consoleRootValue) + { + var consoleRoot = new IntPtr(consoleRootValue); + int visibilityChecks = 0; + using var adapter = new MsalParentWindowAdapter(IntPtr.Zero, false, + () => consoleRoot, + hwnd => + { + Assert.Equal(consoleRoot, hwnd); + visibilityChecks++; + return false; + }, + _ => throw new InvalidOperationException("A progress window should not be created.")); + + object result = adapter.GetWindow(); + + Assert.Null(result); + Assert.Equal(consoleRootValue == 0 ? 0 : 1, visibilityChecks); + } + + [Fact] + public void GetWindow_VisibilityCheckThrows_PropagatesException() + { + var exception = new InvalidOperationException("Visibility check failed."); + using var adapter = new MsalParentWindowAdapter(IntPtr.Zero, true, + () => new IntPtr(42), + _ => throw exception, + _ => throw new InvalidOperationException("A progress window should not be created.")); + + var thrown = Assert.Throws(adapter.GetWindow); + Assert.Same(exception, thrown); + } + + [Fact] + public void Create_ExplicitParent_IsReturned() + { + object parentWindow = new IntPtr(42); + using var adapter = MsalParentWindowAdapter.Create(parentWindow, true); + + object result = adapter.GetWindow(); + Assert.Same(parentWindow, result); + } + + [PosixFact] + public void Create_NoExplicitParent_Posix_ReturnsNull() + { + using var adapter = MsalParentWindowAdapter.Create(IntPtr.Zero); + + object result = adapter.GetWindow(); + Assert.Null(result); + } +} diff --git a/src/Core/Authentication/Entra/MsalAdaptors.cs b/src/Core/Authentication/Entra/MsalAdaptors.cs index 33997fc5e..e169ee838 100644 --- a/src/Core/Authentication/Entra/MsalAdaptors.cs +++ b/src/Core/Authentication/Entra/MsalAdaptors.cs @@ -31,15 +31,30 @@ internal class MsalParentWindowAdapter : IDisposable private readonly bool _createIfMissing; private readonly CancellationTokenSource _cts = new(); + private readonly Func _getConsoleParentWindow; + private readonly Func _isWindowVisible; + private readonly Func _createWindow; + public static MsalParentWindowAdapter Create(object parentWindow, bool createIfMissing = false) { - return new MsalParentWindowAdapter(parentWindow, createIfMissing); + return new MsalParentWindowAdapter(parentWindow, createIfMissing, + GetConsoleParentWindow, User32.IsWindowVisible, ProgressWindow.ShowAndGetHandle); } - private MsalParentWindowAdapter(object parentWindow, bool createIfMissing = false) + internal MsalParentWindowAdapter(object parentWindow, bool createIfMissing, + Func getConsoleParentWindow, Func isWindowVisible, + Func createWindow) { + EnsureArgument.NotNull(getConsoleParentWindow, nameof(getConsoleParentWindow)); + EnsureArgument.NotNull(isWindowVisible, nameof(isWindowVisible)); + EnsureArgument.NotNull(createWindow, nameof(createWindow)); + _parentWindow = parentWindow; _createIfMissing = createIfMissing; + + _getConsoleParentWindow = getConsoleParentWindow; + _isWindowVisible = isWindowVisible; + _createWindow = createWindow; } public object GetWindow() @@ -49,25 +64,38 @@ public object GetWindow() return _parentWindow; } + // See if we can use the console window as a parent. + // We only consider the window if it is valid and visible. + IntPtr consoleParent = _getConsoleParentWindow(); + if (consoleParent != IntPtr.Zero && _isWindowVisible(consoleParent)) + { + return consoleParent; + } + // Create a stub window to use as a parent if (_createIfMissing) { - return ProgressWindow.ShowAndGetHandle(_cts.Token); + return _createWindow(_cts.Token); } + return null; + } + + private static IntPtr GetConsoleParentWindow() + { // On Windows we can try and get the console window parent handle if that exists - if (PlatformUtils.IsWindows()) + if (!PlatformUtils.IsWindows()) { - IntPtr consoleHandle = Kernel32.GetConsoleWindow(); - IntPtr parentHandle = User32.GetAncestor(consoleHandle, GetAncestorFlags.GetRootOwner); - - if (parentHandle != IntPtr.Zero) - { - return parentHandle; - } + return IntPtr.Zero; } - return null; + IntPtr consoleHandle = Kernel32.GetConsoleWindow(); + + // When the parent is using ConPTY (pseudo-terminals) the console + // window may be a fake/stub (as is the case with Windows Terminal). + // This means we need to walk up the ancestor chain to get the actual + // root owner (typically the terminal emulator's window). + return User32.GetAncestor(consoleHandle, GetAncestorFlags.GetRootOwner); } public void Dispose() diff --git a/src/Core/Interop/Windows/Native/User32.cs b/src/Core/Interop/Windows/Native/User32.cs index 8cf05a8d8..7745a10db 100644 --- a/src/Core/Interop/Windows/Native/User32.cs +++ b/src/Core/Interop/Windows/Native/User32.cs @@ -36,6 +36,9 @@ public static IntPtr SetWindowLongPtr(IntPtr hWnd, int nIndex, IntPtr value) [DllImport(LibraryName, SetLastError = true)] public static extern bool GetClientRect(IntPtr hwnd, out RECT lpRect); + [DllImport(LibraryName)] + public static extern bool IsWindowVisible(IntPtr hwnd); + /// /// Retrieves the handle to the ancestor of the specified window. ///