Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Error creating window handle. #4418

Closed
petrochuk opened this issue Nov 11, 2023 · 5 comments
Closed

[Bug] Error creating window handle. #4418

petrochuk opened this issue Nov 11, 2023 · 5 comments

Comments

@petrochuk
Copy link
Contributor

petrochuk commented Nov 11, 2023

Library version used

4.57.0

.NET version

.NET Framework 4.8.9181.0
processArchitecture: X64
osArchitecture: X64

Scenario

PublicClient - desktop app

Is this a new or an existing app?

The app is in production, and I have upgraded to a new version of MSAL

Issue description and reproduction steps

PublicClient.AcquireTokenInteractive call throws System.ComponentModel.Win32Exception "Error creating window handle."

We observed this is happening when our console application is launched by cmd.exe process which is hosted by Windows Terminal.

   at Microsoft.Identity.Client.Platforms.Features.WinFormsLegacyWebUi.WebUI.<AcquireAuthorizationAsync>d__20.MoveNext()
   at Microsoft.Identity.Client.Internal.AuthCodeRequestComponent.<FetchAuthCodeAndPkceInternalAsync>d__7.MoveNext()
   at Microsoft.Identity.Client.Internal.AuthCodeRequestComponent.<FetchAuthCodeAndPkceVerifierAsync>d__4.MoveNext()
   at Microsoft.Identity.Client.Internal.Requests.InteractiveRequest.<GetTokenResponseAsync>d__11.MoveNext()
   at Microsoft.Identity.Client.Internal.Requests.InteractiveRequest.<ExecuteAsync>d__9.MoveNext()
   at Microsoft.Identity.Client.Internal.Requests.RequestBase.<RunAsync>d__12.MoveNext()
   at Microsoft.Identity.Client.ApiConfig.Executors.PublicClientExecutor.<ExecuteAsync>d__2.MoveNext()
   at bolt.authentication.store.AuthTokenStore.<AcquireTokenAsync>d__20.MoveNext()

Identity provider

Microsoft Entra ID (Work and School accounts and Personal Microsoft accounts)

Solution and workarounds

use cmd.exe outside of Windows Terminal

sounds like related to this one: AzureAD/MSAL.PS#58

@petrochuk petrochuk added needs attention Delete label after triage untriaged Do not delete. Needed for Automation labels Nov 11, 2023
@neha-bhargava neha-bhargava added public-client bug scenario:Desktop and removed untriaged Do not delete. Needed for Automation needs attention Delete label after triage labels Nov 16, 2023
@neha-bhargava
Copy link
Contributor

Was this scenario working fine with the old MSAL version before upgrading? Which version were you using before?

Or it started happening when using cmd.exe outside of Windows terminal?

@bgavrilMS
Copy link
Member

Hi @petrochuk -

I don't think we made any changes in that area of the code. How do you configure PublicClientApplicaiton and AcquireTokenInteractive?

It is possible that a Windows update introduced some regression. But in general Terminal's way of handling windows is pretty different. Read on..

  1. we recommend moving to WAM, see https://aka.ms/msal-net-wam and read about window handles (they are mandatory with WAM) https://learn.microsoft.com/en-us/entra/msal/dotnet/acquiring-tokens/desktop-mobile/wam#parent-window-handles

  2. even with the embedded browser, we recommend we provide a window handle to your console. Otherwise MSAL will use the center of the screen. See https://learn.microsoft.com/en-us/entra/msal/dotnet/acquiring-tokens/desktop-mobile/wam#parent-window-handles which has special code for dealing with Terminal.

@petrochuk
Copy link
Contributor Author

@bgavrilMS, thanks for your reply.

  1. We do use WAM by default, but we also provide users with ability to do interactive auth in case they need to login to different tenant.
  2. We also do provide valid window handle. Our code walks process tree to find topmost parent process with valid window handle and passes it to PublicClientApplicationBuilder.WithParentActivityOrWindow

@bgavrilMS
Copy link
Member

bgavrilMS commented Nov 17, 2023

Hi @petrochuk, does the issue repro if you use the logic below for finding handle for terminal window?

enum GetAncestorFlags
{   
    GetParent = 1,
    GetRoot = 2,
    /// <summary>
    /// Retrieves the owned root window by walking the chain of parent and owner windows returned by GetParent.
    /// </summary>
    GetRootOwner = 3
}

/// <summary>
/// Retrieves the handle to the ancestor of the specified window.
/// </summary>
/// <param name="hwnd">A handle to the window whose ancestor is to be retrieved.
/// If this parameter is the desktop window, the function returns NULL. </param>
/// <param name="flags">The ancestor to be retrieved.</param>
/// <returns>The return value is the handle to the ancestor window.</returns>
[DllImport("user32.dll", ExactSpelling = true)]
static extern IntPtr GetAncestor(IntPtr hwnd, GetAncestorFlags flags);

[DllImport("kernel32.dll")]
static extern IntPtr GetConsoleWindow();

// This is your window handle!
public IntPtr GetConsoleOrTerminalWindow()
{
    IntPtr consoleHandle = GetConsoleWindow();
    IntPtr handle = GetAncestor(consoleHandle, GetAncestorFlags.GetRootOwner );
    
    return handle;
}

@petrochuk
Copy link
Contributor Author

We used the code below, but your suggestion looks much more efficient. I am closing the issue.

    /// <summary>
    /// Gets the parent process of a specified process.
    /// </summary>
    /// <param name="handle">The process handle.</param>
    /// <returns>An instance of the Process class.</returns>
    public static Process GetParentProcess(IntPtr handle)
    {
        var pbi = default(ProcessInformation);
        int status = NtQueryInformationProcess(handle, 0, ref pbi, Marshal.SizeOf(pbi), out var _);
        if (status != 0)
            throw new Win32Exception(status);

        try
        {
            return Process.GetProcessById(pbi.InheritedFromUniqueProcessId.ToInt32());
        }
        catch (ArgumentException)
        {
            // not found
#pragma warning disable CS8603 // Possible null reference return.
            return null;
#pragma warning restore CS8603 // Possible null reference return.
        }
    }

    /// <summary>
    /// Searches the process tree for the first process with a window.
    /// </summary>
    /// <returns></returns>
    public static IntPtr GetProcessMainWindowHandle()
    {
        // Walk up the process tree until we find a process with a window
        var parentProcess = GetParentProcess();
        while (parentProcess != null && parentProcess.MainWindowHandle == IntPtr.Zero)
        {
            parentProcess = GetParentProcess(parentProcess.Id);
        }

#pragma warning disable CS8602 // Dereference of a possibly null reference.
        return parentProcess.MainWindowHandle;
#pragma warning restore CS8602 // Dereference of a possibly null reference.
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants