[Bug] Crashing Microsoft.Identity.Client.Broker - WithBrokerPreview()

#3654

src/client/Microsoft.Identity.Client.Broker/RuntimeBroker.cs

@@ -109,7 +109,10 @@ private static void OnProcessExit(object sender, EventArgs e)
 
             if (authenticationRequestParameters?.Account?.HomeAccountId?.ObjectId != null)
             {
-                using (var authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
+                using (var authParams = WamAdapters.GetCommonAuthParameters(
+                    authenticationRequestParameters, 
+                    _wamOptions.MsaPassthrough, 
+                    _logger))
                 {
                     using (var readAccountResult = await s_lazyCore.Value.ReadAccountByIdAsync(
                     authenticationRequestParameters.Account.HomeAccountId.ObjectId,
@@ -161,7 +164,10 @@ private static void OnProcessExit(object sender, EventArgs e)
             var cancellationToken = authenticationRequestParameters.RequestContext.UserCancellationToken;
             Debug.Assert(s_lazyCore.Value != null, "Should not call this API if msal runtime init failed");
 
-            using (var authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
+            using (var authParams = WamAdapters.GetCommonAuthParameters(
+                authenticationRequestParameters, 
+                _wamOptions.MsaPassthrough,
+                _logger))
             {
                 //Login Hint
                 string loginHint = authenticationRequestParameters.LoginHint ?? authenticationRequestParameters?.Account?.Username;
@@ -193,7 +199,10 @@ private static void OnProcessExit(object sender, EventArgs e)
 
             _logger.Verbose("[WamBroker] Signing in with the default user account.");
 
-            using (var authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
+            using (var authParams = WamAdapters.GetCommonAuthParameters(
+                authenticationRequestParameters, 
+                _wamOptions.MsaPassthrough,
+                _logger))
             {
                 using (NativeInterop.AuthResult result = await s_lazyCore.Value.SignInAsync(
                         _parentHandle,
@@ -230,7 +239,10 @@ private IntPtr GetParentWindow(CoreUIParent uiParent)
 
             _logger.Verbose("[WamBroker] Acquiring token silently.");
 
-            using (var authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
+            using (var authParams = WamAdapters.GetCommonAuthParameters(
+                authenticationRequestParameters, 
+                _wamOptions.MsaPassthrough,
+                _logger))
             {
                 using (var readAccountResult = await s_lazyCore.Value.ReadAccountByIdAsync(
                     acquireTokenSilentParameters.Account.HomeAccountId.ObjectId,
@@ -274,7 +286,10 @@ private IntPtr GetParentWindow(CoreUIParent uiParent)
 
             _logger.Verbose("[WamBroker] Acquiring token silently for default account.");
 
-            using (var authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
+            using (var authParams = WamAdapters.GetCommonAuthParameters(
+                authenticationRequestParameters, 
+                _wamOptions.MsaPassthrough,
+                _logger))
             {
                 using (NativeInterop.AuthResult result = await s_lazyCore.Value.SignInSilentlyAsync(
                         authParams,
@@ -300,7 +315,10 @@ private IntPtr GetParentWindow(CoreUIParent uiParent)
 
             _logger.Verbose("[WamBroker] Acquiring token with Username Password flow.");
 
-            using (AuthParameters authParams = WamAdapters.GetCommonAuthParameters(authenticationRequestParameters, _wamOptions.MsaPassthrough))
+            using (AuthParameters authParams = WamAdapters.GetCommonAuthParameters(
+                authenticationRequestParameters, 
+                _wamOptions.MsaPassthrough,
+                _logger))
             {
                 authParams.Properties["MSALRuntime_Username"] = acquireTokenByUsernamePasswordParameters.Username;
                 authParams.Properties["MSALRuntime_Password"] = acquireTokenByUsernamePasswordParameters.Password;
@@ -424,7 +442,7 @@ public bool IsBrokerInstalledAndInvokable(AuthorityType authorityType)
                 return false;
             }
 
-            _logger.Verbose($"[WAM Broker] MsalRuntime init succesful.");
+            _logger.Verbose($"[WAM Broker] MsalRuntime init successful.");
             return true;
         }
     }

src/client/Microsoft.Identity.Client.Broker/WamAdapters.cs

@@ -130,10 +130,15 @@ private enum ResponseStatus
         /// </summary>
         /// <param name="authenticationRequestParameters"></param>
         /// <param name="isMsaPassthrough"></param>
+        /// <param name="logger"></param>
         public static NativeInterop.AuthParameters GetCommonAuthParameters(
             AuthenticationRequestParameters authenticationRequestParameters, 
-            bool isMsaPassthrough)
+            bool isMsaPassthrough,
+            ILoggerAdapter logger)
         {
+            logger.Info("[WamBroker] Validating Common Auth Parameters.");
+            ValidateAuthParams(authenticationRequestParameters, logger);
+
             var authParams = new NativeInterop.AuthParameters
                 (authenticationRequestParameters.AppConfig.ClientId,
                 authenticationRequestParameters.Authority.AuthorityInfo.CanonicalAuthority.ToString());
@@ -171,6 +176,8 @@ private enum ResponseStatus
 
             AddPopParams(authenticationRequestParameters, authParams);
 
+            logger.Info("[WamBroker] Acquired Common Auth Parameters.");
+
             return authParams;
         }
 
@@ -281,5 +288,28 @@ private static string GetExpectedRedirectUri(string clientId)
         {
             return $"ms-appx-web://microsoft.aad.brokerplugin/{clientId}";
         }
+
+        /// <summary>
+        /// Validate common auth params
+        /// </summary>
+        /// <param name="authenticationRequestParameters"></param>
+        /// <param name="logger"></param>
+        /// <exception cref="MsalClientException"></exception>
+        public static void ValidateAuthParams(
+            AuthenticationRequestParameters authenticationRequestParameters,
+            ILoggerAdapter logger)
+        {
+            //MSAL Runtime throws an ApiContractViolation Exception with Tag: 0x2039c1cb (InvalidArg)
+            //When no scopes are passed, this will check if user is passing scopes
+            if (!authenticationRequestParameters.HasScopes)
+            {
+                logger.Error($"[WamBroker] {MsalError.WamScopesRequired} " +
+                    $"{MsalErrorMessage.ScopesRequired}");
+
+                throw new MsalClientException(
+                    MsalError.WamScopesRequired,
+                    MsalErrorMessage.ScopesRequired);
+            }
+        }
     }
 }

src/client/Microsoft.Identity.Client/MsalError.cs

@@ -1016,6 +1016,12 @@ public static class MsalError
         /// </summary>
         public const string WamPickerError = "wam_interactive_picker_error";
 
+        /// <summary>
+        /// <para>What happens?</para>No scopes have been requested
+        /// <para>Mitigation</para>At least one scope must be specified for MSAL Runtime WAM
+        /// </summary>
+        public const string WamScopesRequired = "scopes_required_wam";
+
         /// <summary>
         /// <para>What happens?</para>The embedded browser cannot be started because a runtime component is missing.
         /// <para>Mitigation</para>"The embedded browser needs WebView2 runtime to be installed. An end user of the app can download and install the WebView2 runtime from https://go.microsoft.com/fwlink/p/?LinkId=2124703 and restart the app.

src/client/Microsoft.Identity.Client/Platforms/Android/Broker/AndroidContentProviderBroker.cs

@@ -340,7 +340,7 @@ private async Task<Bundle> PerformContentResolverOperationAsync(ContentResolverO
 
             if (resultBundle != null)
             {
-                _logger.Verbose($"[Android broker] Content resolver operation completed succesfully. Operation: {operation} URI: {contentResolverUri}");
+                _logger.Verbose($"[Android broker] Content resolver operation completed successfully. Operation: {operation} URI: {contentResolverUri}");
             }
 
             return resultBundle;

tests/Microsoft.Identity.Test.Common/Mocks/MsalMockHelpers.cs

@@ -35,7 +35,7 @@ internal static class MsalMockHelpers
         }
 
         /// <summary>
-        /// Configures a web ui that returns a succesfull result
+        /// Configures a web ui that returns a successfull result
         /// </summary>
         public static void ConfigureMockWebUI(
             this IServiceBundle serviceBundle, 

tests/Microsoft.Identity.Test.Unit/Throttling/ThrottlingAcceptanceTests.cs

@@ -458,7 +458,7 @@ await app.AcquireTokenSilent(TestConstants.s_scope, account).ExecuteAsync()
                 await app.AcquireTokenSilent(singleScope, account).WithForceRefresh(true).ExecuteAsync()
                       .ConfigureAwait(false);
 
-                // there will still be an entry here because the refresh token written by the first succesful ATS
+                // there will still be an entry here because the refresh token written by the first successful ATS
                 // is different from the initial RT
                 AssertThrottlingCacheEntryCount(throttlingManager, uiRequiredEntryCount: 1);
             }

tests/devapps/WAM/NetCoreWinFormsWam/Form1.cs

@@ -273,9 +273,11 @@ private async Task<AuthenticationResult> RunAtsAsync(IPublicClientApplication pc
         private string[] GetScopes()
         {
             string[] result = null;
+
             cbxScopes.Invoke((MethodInvoker)delegate
             {
-                result = cbxScopes.Text.Split(' ');
+                if(!string.IsNullOrWhiteSpace(cbxScopes.Text))
+                    result = cbxScopes.Text.Split(' ');
             });
 
             return result;

tests/devapps/WAM/NetFrameworkWam/Form1.cs

@@ -237,7 +237,8 @@ private string[] GetScopes()
             string[] result = null;
             cbxScopes.Invoke((MethodInvoker)delegate
             {
-                result = cbxScopes.Text.Split(' ');
+                if (!string.IsNullOrWhiteSpace(cbxScopes.Text))
+                    result = cbxScopes.Text.Split(' ');
             });
 
             return result;