AzureAD / microsoft-authentication-library-for-dotnet

Microsoft Authentication Library (MSAL) for .NET
https://aka.ms/msal-net
MIT License
1.37k stars 338 forks source link

When running code to acquire access token AcquireTokenInteractive never returns never #4598

Open dgxhubbard opened 7 months ago

dgxhubbard commented 7 months ago

Library version used

4.59.0

.NET version

.net 7

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

I run the code in my wpf application and the call to AcquireTokenInteractive never returns and does not show any UI to choose a user. I place the same code in a command line app and get UI to select user and send test email.

Relevant code snippets

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using System.Security.Cryptography;
using System.Threading;
using System.Threading.Tasks;

using Microsoft.Identity.Client;

namespace TestAccessToken
{
    internal class Program
    {
        public static string ClientId = "c266bdd1-d87e-44d5-a5db-f153234068a2";

        public static string Tenant = "common";
        public static string Instance = "https://login.microsoftonline.com/";

        public static string RedirectUri = "http://localhost";
        public static string Authority = "https://login.microsoftonline.com/common/";

        static async Task Main ( string [] args )
        {
            var stoppingToken = new CancellationToken ();
            string accessToken = null;

            AuthenticationResult authenticationResult = null;

            Console.WriteLine ( "Press any key to start the authentication process." );
            await Task.Run ( Console.ReadKey ).WaitAsync ( stoppingToken );

            var scopes =
                new List<string> ()
                {
                            "email",
                            "offline_access",
                            //"https://outlook.office.com/IMAP.AccessAsUser.All", // Only needed for IMAP
                            //"https://outlook.office.com/POP.AccessAsUser.All",  // Only needed for POP
                            "https://outlook.office.com/SMTP.Send", // Only needed for SMTP
                };

            var clientApp = 
                PublicClientApplicationBuilder.
                    Create ( ClientId ).
                    WithAuthority ( Authority ).
                    WithRedirectUri ( RedirectUri ).
                    Build ();

            if ( clientApp == null )
                throw new NullReferenceException ( "FAILED to create client app builder" );

            var accounts = await clientApp.GetAccountsAsync ();

            IAccount firstAccount = null;

            if ( accounts != null )
                firstAccount = accounts.FirstOrDefault ();

            var success = false;
            try
            {
                authenticationResult = await clientApp.AcquireTokenSilent ( scopes, firstAccount ).ExecuteAsync ();

                if ( authenticationResult != null )
                    accessToken = authenticationResult.AccessToken;

                success = true;
            }
            catch ( Exception ex )
            {
                Console.WriteLine ( "Could not acquire token silent " + ex.Message );
            }

            if ( !success )
            {
                try
                {
                    authenticationResult = await clientApp.AcquireTokenInteractive ( scopes )
                        .ExecuteAsync ();

                    if ( authenticationResult != null )
                        accessToken = authenticationResult.AccessToken;
                }
                catch ( MsalException msalex )
                {
                    Console.WriteLine ( "Error Acquiring Token" + msalex.Message );
                }
            }

        }
    }
}

Expected behavior

I expect the code to allow user to login and send test email. Right now the command line app works but it does not appear that a token is stored because AcquireTokenSilent always throws an exception. The code to get first account also always returns null:

        var accounts = await clientApp.GetAccountsAsync ();

        IAccount firstAccount = null;

        if ( accounts != null )
            firstAccount = accounts.FirstOrDefault ();

Identity provider

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

Regression

4.54.0

Solution and workarounds

None

dgxhubbard commented 7 months ago

We also had this class for a cache helper is something like this still needed

   public static class AzureTokenCacheHelper
   {
       static AzureTokenCacheHelper ()
       {
           try
           {
               string folder = Environment.GetFolderPath ( Environment.SpecialFolder.ApplicationData );

               // Combine the base folder with your specific folder....
               string localFolder = Path.Combine ( folder, "Local" );

               // CreateDirectory will check if every folder in path exists and, if not, create them.
               // If all folders exist then CreateDirectory will do nothing.
               Directory.CreateDirectory ( localFolder );

               var localAppData = Environment.GetFolderPath ( Environment.SpecialFolder.LocalApplicationData );

               // For packaged desktop apps (MSIX packages, also called desktop bridge) the executing assembly folder is read-only. 
               // In that case we need to use Windows.Storage.ApplicationData.Current.LocalCacheFolder.Path + "\msalcache.bin" 
               // which is a per-app read/write folder for packaged apps.
               // See https://docs.microsoft.com/windows/msix/desktop/desktop-to-uwp-behind-the-scenes

               CacheFilePath = Path.Combine ( localAppData, ".msalcache.bin3" );
           }
           catch ( System.InvalidOperationException )
           {
               // Fall back for an unpackaged desktop app
               CacheFilePath = System.Reflection.Assembly.GetExecutingAssembly ().Location + ".msalcache.bin3";
           }
       }

       /// <summary>
       /// Path to the token cache
       /// </summary>
       public static string CacheFilePath { get; private set; }

       private static readonly object FileLock = new object ();

       public static void BeforeAccessNotification ( TokenCacheNotificationArgs args )
       {
           lock ( FileLock )
           {
               args.TokenCache.DeserializeMsalV3 ( File.Exists ( CacheFilePath )
                       ? ProtectedData.Unprotect ( File.ReadAllBytes ( CacheFilePath ),
                                                null,
                                                DataProtectionScope.CurrentUser )
                       : null );
           }
       }

       public static void AfterAccessNotification ( TokenCacheNotificationArgs args )
       {
           // if the access operation resulted in a cache update
           if ( args.HasStateChanged )
           {
               lock ( FileLock )
               {
                   // reflect changes in the persistent store
                   File.WriteAllBytes (
                       CacheFilePath,
                       ProtectedData.Protect ( args.TokenCache.SerializeMsalV3 (),
                                               null,
                                               DataProtectionScope.CurrentUser ) );
               }
           }
       }

       public static void EnableSerialization ( ITokenCache tokenCache )
       {
           tokenCache.SetBeforeAccess ( BeforeAccessNotification );
           tokenCache.SetAfterAccess ( AfterAccessNotification );
       }
   }
dgxhubbard commented 7 months ago

I have reproduced the error in a wpf app. Using latest VS 2022, WPF on .net 7 create a WPF application and in MainWindow.xaml code below to the grid. Then in MainWindow.xaml.cs use other code below. Add Microsoft.Identity.Client and Microsoft.Identity.Client.Broker both version 4.59.0. Run and code will not return.

MainWindow.xaml


        <Button
            Width="75" Height="25"
            Content="Get Token"
            Grid.Row="0" Grid.Column="0"
            Click="Button_Click"    
            />

MainWindow.xaml.cs

using System.Security.Principal;
using System.Text;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;

using System.Threading;
using System.Threading.Tasks;

using Microsoft.Identity.Client;

namespace WpfApp1
{
    /// <summary>
    /// Interaction logic for MainWindow.xaml
    /// </summary>
    public partial class MainWindow : Window
    {

        public static string ClientId = "add your client id here";

        public static string Tenant = "common";
        public static string Instance = "https://login.microsoftonline.com/";

        public static string RedirectUri = "http://localhost";
        public static string Authority = "https://login.microsoftonline.com/common/";

        public MainWindow ()
        {
            InitializeComponent ();
        }

        private void Button_Click ( object sender, RoutedEventArgs e )
        {

            GetAccessTokenAsync ().GetAwaiter ().GetResult ();

        }

        private async Task GetAccessTokenAsync ()
        { 

            var stoppingToken = new CancellationToken ();
            string accessToken = null;

            AuthenticationResult authenticationResult = null;

            var scopes =
                new List<string> ()
                {
                            "email",
                            "offline_access",
                            //"https://outlook.office.com/IMAP.AccessAsUser.All", // Only needed for IMAP
                            //"https://outlook.office.com/POP.AccessAsUser.All",  // Only needed for POP
                            "https://outlook.office.com/SMTP.Send", // Only needed for SMTP
                };

            var clientApp =
                PublicClientApplicationBuilder.
                    Create ( ClientId ).
                    WithAuthority ( Authority ).
                    WithRedirectUri ( RedirectUri ).
                    Build ();

            if ( clientApp == null )
                throw new NullReferenceException ( "FAILED to create client app builder" );

            var accounts = await clientApp.GetAccountsAsync ();

            IAccount firstAccount = null;

            if ( accounts != null )
                firstAccount = accounts.FirstOrDefault ();

            var success = false;
            try
            {
                authenticationResult = await clientApp.AcquireTokenSilent ( scopes, firstAccount ).ExecuteAsync ();

                if ( authenticationResult != null )
                    accessToken = authenticationResult.AccessToken;

                success = true;
            }
            catch ( Exception ex )
            {
                Console.WriteLine ( "Could not acquire token silent " + ex.Message );
            }

            if ( !success )
            {
                try
                {
                    authenticationResult = await clientApp.AcquireTokenInteractive ( scopes )
                        .ExecuteAsync ();

                    if ( authenticationResult != null )
                        accessToken = authenticationResult.AccessToken;
                }
                catch ( MsalException msalex )
                {
                    Console.WriteLine ( "Error Acquiring Token" + msalex.Message );
                }
            }

        }
    }
}
dgxhubbard commented 7 months ago

I found this code from @jstedfast but this does not work with 4.59.0 either

bgavrilMS commented 7 months ago

You should see the system browser pop up. If the system browser is already up, it may be that it opens a new tab in there. Normally focus is given to the app.

Still, we recommend folks to use the Windows broker instead of the browser. See https://aka.ms/msal-net-wam - it'll be just a few lines of changes.

dgxhubbard commented 7 months ago

@bgavrilMS the browser never pops up it used on older version now it just locks up

dgxhubbard commented 7 months ago

Replacing the MainWindow.xaml.cs code with code below now it locks up in the call to AcquireTokenSilent. I would just like to get the user to be able to login to azure how do I do this from a WPF app?

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using System.Security.Cryptography;
using System.Threading;
using System.Threading.Tasks;

using System.Security.Principal;
using System.Text;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;

using System.Threading;
using System.Threading.Tasks;

using Microsoft.Identity.Client;
using static System.Windows.Forms.Design.AxImporter;
using System.Runtime.InteropServices;

namespace WpfApp1
{
    /// <summary>
    /// Interaction logic for MainWindow.xaml
    /// </summary>
    public partial class MainWindow : Window
    {

        public static string ClientId = "c266bdd1-d87e-44d5-a5db-f153234068a2";

        public static string Tenant = "common";
        public static string Instance = "https://login.microsoftonline.com/";

        public static string RedirectUri = "https://login.microsoftonline.com/common/oauth2/nativeclient";
        public static string Authority = "https://login.microsoftonline.com/common/";

        const string ExchangeAccount = "dhubbard@cybermetrics.com";

        public MainWindow ()
        {
            InitializeComponent ();
        }

        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;
        }

        private void Button_Click ( object sender, RoutedEventArgs e )
        {

            GetAccessTokenAsync ().GetAwaiter ().GetResult ();

        }

        private async Task GetAccessTokenAsync ()
        { 

            var cancellationToken = new CancellationToken ();
            string accessToken = null;

            AuthenticationResult authenticationResult = null;

            var scopes =
                new List<string> ()
                {
                            "email",
                            "offline_access",
                            "https://outlook.office.com/IMAP.AccessAsUser.All", // Only needed for IMAP
                                                                                //"https://outlook.office.com/POP.AccessAsUser.All",  // Only needed for POP
                                                                                //"https://outlook.office.com/SMTP.Send", // Only needed for SMTP
                };

            BrokerOptions options = new BrokerOptions ( BrokerOptions.OperatingSystems.Windows );
            options.Title = "My Awesome Application";

            IPublicClientApplication app =
                PublicClientApplicationBuilder.Create ( ClientId )
                .WithDefaultRedirectUri ()
                .WithParentActivityOrWindow ( GetConsoleOrTerminalWindow )
                .WithBroker ( options )
                .Build ();

            AuthenticationResult result = null;

            // Try to use the previously signed-in account from the cache
            IEnumerable<IAccount> accounts = await app.GetAccountsAsync ();
            IAccount existingAccount = accounts.FirstOrDefault ();

            try
            {
                if ( existingAccount != null )
                {
                    result = await app.AcquireTokenSilent ( scopes, existingAccount ).ExecuteAsync ();
                }
                // Next, try to sign in silently with the account that the user is signed into Windows
                else
                {
                    result = await app.AcquireTokenSilent ( scopes, PublicClientApplication.OperatingSystemAccount )
                                        .ExecuteAsync ();
                }
            }
            // Can't get a token silently, go interactive
            catch ( MsalUiRequiredException ex )
            {
                result = await app.AcquireTokenInteractive ( scopes ).ExecuteAsync ();
            }

            /*
            var clientApp =
                PublicClientApplicationBuilder.
                    Create ( ClientId ).
                    WithAuthority ( Authority ).
                    WithRedirectUri ( RedirectUri ).
                    Build ();

            if ( clientApp == null )
                throw new NullReferenceException ( "FAILED to create client app builder" );

            var accounts = await clientApp.GetAccountsAsync ();

            IAccount firstAccount = null;

            if ( accounts != null )
                firstAccount = accounts.FirstOrDefault ();

            var success = false;
            try
            {
                authenticationResult = await clientApp.AcquireTokenSilent ( scopes, firstAccount ).ExecuteAsync ();

                if ( authenticationResult != null )
                    accessToken = authenticationResult.AccessToken;

                success = true;
            }
            catch ( Exception ex )
            {
                Console.WriteLine ( "Could not acquire token silent " + ex.Message );
            }

            if ( !success )
            {
                try
                {
                    authenticationResult = 
                        await clientApp
                        .AcquireTokenInteractive ( scopes )
                        .WithLoginHint ( ExchangeAccount )
                        .ExecuteAsync ();

                    if ( authenticationResult != null )
                        accessToken = authenticationResult.AccessToken;
                }
                catch ( MsalException msalex )
                {
                    Console.WriteLine ( "Error Acquiring Token" + msalex.Message );
                }
            }

            */

        }
    }
}
mcgeeky commented 7 months ago

Possibly related to #4601 ?

dgxhubbard commented 7 months ago

Our code used to work with Identity Client 4.54.0.0 and Identity Client Broker 4.54.0.0

mcgeeky commented 7 months ago

4.54 was dependent upon Microsoft.Identity.Client.NativeInterop 0.13.8. perhaps something has changed in NativeInterop v0.14.1 that is causing the lock up?

dgxhubbard commented 7 months ago

@bgavrilMS We used microsoft.data.sqlclient 5.1.1 and micrsoft.client.identity and microsoft.client.identity.broker then microsoft.data.sqlclientm 5.1.1 had a vulnerability so we had to upgrade then sign on to azure locked up the way we used to use it and the preferred way. The way we use it now does not require Win32 calls which should be the preferred way

dgxhubbard commented 7 months ago

@bgavrilMS Also as noted at the start of this thread our code works in a console app, but as noted acquiring a token silently never works on multiple calls.

https://github.com/Azure-Samples/ms-identity-dotnet-desktop-tutorial

softlion commented 6 months ago

same issue here with 4.59 on Android with maui on net8.
Cancelling the login closes the window but the await never returns.

The same app/code on iOS is working as expected.

Rolling back to 4.54 does not work either. Nor 4.50.

THE SAME CODE DOES WORK CORRECTLY ON XAMARIN ANDROID. But I migrated to Maui.

        var adClient = PublicClientApplicationBuilder
                .Create(appSettings.ClientId)
                .WithRedirectUri(appSettings.RedirectUri)
                .WithAuthority(appSettings.AdAuthorityUrl)
#if DEBUG
                .WithDebugLoggingCallback(Microsoft.Identity.Client.LogLevel.Verbose, true, true)
                .WithLogging(this, true)
#endif
#if IOS
                .WithParentActivityOrWindow(Platform.GetCurrentUIViewController)
                .WithIosKeychainSecurityGroup(Foundation.NSBundle.MainBundle.BundleIdentifier)
#endif
#if ANDROID
                .WithParentActivityOrWindow(() => Platform.CurrentActivity)        
#endif
            .Build();

        var query = adClient.AcquireTokenInteractive(scopes)
            .WithUseEmbeddedWebView(true)
            .WithPrompt(Prompt.ForceLogin);

        var result = await query.ExecuteAsync(); //Displays the login UI as expected.

       //But never go past that point. No exception, no log. Even when choosing the back button to cancel the interactive login.