Skip to content

Commit 134e622

Browse files
committed
msauth: always set the parent window for MSAL
Always set the parent window handle for MSAL on Windows. Previously we only set this if provided a handle by the user/config. Now however we must always try and provide a handle because using the new MSALRuntime-based Windows broker means we must do so - MSAL no longer provides us with a 'dummy' handle to use. Use the parent console window handle, as recommended by the MSAL docs: https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/wam#parent-window-handles
1 parent ac5289a commit 134e622

File tree

3 files changed

+59
-5
lines changed

3 files changed

+59
-5
lines changed

src/shared/Core/Authentication/MicrosoftAuthentication.cs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.IO;
44
using System.Net.Http;
55
using System.Threading.Tasks;
6+
using GitCredentialManager.Interop.Windows.Native;
67
using Microsoft.Identity.Client;
78
using Microsoft.Identity.Client.Extensions.Msal;
89

@@ -223,12 +224,25 @@ private async Task<IPublicClientApplication> CreatePublicClientApplicationAsync(
223224
appBuilder.WithLogging(OnMsalLogMessage, LogLevel.Verbose, enablePiiLogging, false);
224225
}
225226

226-
// If we have a parent window ID we should tell MSAL about it so it can parent any authentication dialogs
227-
// correctly. We only support this on Windows right now as MSAL only supports embedded/dialogs on Windows.
228-
if (PlatformUtils.IsWindows() && !string.IsNullOrWhiteSpace(Context.Settings.ParentWindowId) &&
229-
int.TryParse(Context.Settings.ParentWindowId, out int hWndInt) && hWndInt > 0)
227+
// On Windows we should set the parent window handle for the authentication dialogs
228+
// so that they are displayed as a child of the correct window.
229+
if (PlatformUtils.IsWindows())
230230
{
231-
appBuilder.WithParentActivityOrWindow(() => new IntPtr(hWndInt));
231+
// If we have a parent window ID then use that, otherwise use the hosting terminal window.
232+
IntPtr parentHandle;
233+
if (!string.IsNullOrWhiteSpace(Context.Settings.ParentWindowId) &&
234+
int.TryParse(Context.Settings.ParentWindowId, out int hWndInt) && hWndInt > 0)
235+
{
236+
parentHandle = new IntPtr(hWndInt);
237+
}
238+
else
239+
{
240+
IntPtr consoleHandle = Kernel32.GetConsoleWindow();
241+
parentHandle = User32.GetAncestor(consoleHandle, GetAncestorFlags.GetRootOwner);
242+
}
243+
244+
Context.Trace.WriteLine($"Using parent window ID '{parentHandle}' for MSAL authentication dialogs.");
245+
appBuilder.WithParentActivityOrWindow(() => parentHandle);
232246
}
233247

234248
// Configure the broker if enabled

src/shared/Core/Interop/Windows/Native/Kernel32.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,16 @@ public static extern bool SetConsoleMode(
251251
/// </returns>
252252
[DllImport(LibraryName, CallingConvention = CallingConvention.StdCall, CharSet = CharSet.Unicode, SetLastError = true)]
253253
public static extern IntPtr LocalFree(IntPtr ptr);
254+
255+
/// <summary>
256+
/// Retrieves the window handle used by the console associated with the calling process.
257+
/// </summary>
258+
/// <returns>
259+
/// The return value is a handle to the window used by the console associated with the calling process or
260+
/// NULL if there is no such associated console.
261+
/// </returns>
262+
[DllImport("kernel32.dll", SetLastError = true)]
263+
public static extern IntPtr GetConsoleWindow();
254264
}
255265

256266
[Flags]

src/shared/Core/Interop/Windows/Native/User32.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,36 @@ public static IntPtr SetWindowLongPtr(IntPtr hWnd, int nIndex, IntPtr value)
3535

3636
[DllImport(LibraryName, SetLastError = true)]
3737
public static extern bool GetClientRect(IntPtr hwnd, out RECT lpRect);
38+
39+
/// <summary>
40+
/// Retrieves the handle to the ancestor of the specified window.
41+
/// </summary>
42+
/// <param name="hwnd">
43+
/// A handle to the window whose ancestor is to be retrieved.
44+
/// If this parameter is the desktop window, the function returns NULL.
45+
/// </param>
46+
/// <param name="flags">The ancestor to be retrieved.</param>
47+
/// <returns>The return value is the handle to the ancestor window.</returns>
48+
[DllImport("user32.dll", SetLastError = true)]
49+
public static extern IntPtr GetAncestor(IntPtr hwnd, GetAncestorFlags flags);
50+
}
51+
52+
public enum GetAncestorFlags
53+
{
54+
/// <summary>
55+
/// Retrieves the parent window. This does not include the owner, as it does with the GetParent function.
56+
/// </summary>
57+
GetParent = 1,
58+
59+
/// <summary>
60+
/// Retrieves the root window by walking the chain of parent windows.
61+
/// </summary>
62+
GetRoot = 2,
63+
64+
/// <summary>
65+
/// Retrieves the owned root window by walking the chain of parent and owner windows returned by GetParent.
66+
/// </summary>
67+
GetRootOwner = 3
3868
}
3969

4070
[StructLayout(LayoutKind.Sequential)]

0 commit comments

Comments
 (0)