Merge pull request #1010 from AzureAD/markzuber/synccontext

MarkZuber · web-flow · commit 915ee31e2f3f · 2019-03-25T13:06:36.000-07:00
fix windows ui deadlock with sync context [1009]
diff --git a/src/Microsoft.Identity.Client/ApiConfig/AcquireTokenInteractiveParameterBuilder.cs b/src/Microsoft.Identity.Client/ApiConfig/AcquireTokenInteractiveParameterBuilder.cs
@@ -74,10 +74,17 @@ internal static AcquireTokenInteractiveParameterBuilder Create(
             object parent)
         {
             return new AcquireTokenInteractiveParameterBuilder(publicClientApplicationExecutor)
+                .WithCurrentSynchronizationContext()
                 .WithScopes(scopes)
                 .WithParent(parent);
         }
 
+        internal AcquireTokenInteractiveParameterBuilder WithCurrentSynchronizationContext()
+        {
+            Parameters.UiParent.SynchronizationContext = SynchronizationContext.Current;
+            return this;
+        }
+
         /// <summary>
         /// Specifies if the public client application should used an embedded web browser
         /// or the system default browser
@@ -154,7 +161,8 @@ protected override void Validate()
 #if ANDROID
             if (_ownerWindow is Activity activity)
             {
-                Parameters.UiParent.SetAndroidActivity(activity);
+                Parameters.UiParent.Activity = activity;
+                Parameters.UiParent.CallerActivity = activity;
             }
             else
             {
@@ -163,17 +171,17 @@ protected override void Validate()
 #elif iOS
             if(_ownerWindow is UIViewController uiViewController)
             {
-                Parameters.UiParent.SetUIViewController(uiViewController);
+                Parameters.UiParent.CallerViewController = uiViewController;
             }
 
 #elif DESKTOP
             if (_ownerWindow is IWin32Window win32Window)
             {
-                Parameters.UiParent.SetOwnerWindow(win32Window);
+                Parameters.UiParent.OwnerWindow = win32Window;
             }
             else if (_ownerWindow is IntPtr intPtrWindow)
             {
-                Parameters.UiParent.SetOwnerWindow(intPtrWindow);
+                Parameters.UiParent.OwnerWindow = intPtrWindow;
             }
             // It's ok on Windows Desktop to not have an owner window, the system will just center on the display
             // instead of a parent.
diff --git a/src/Microsoft.Identity.Client/ApiConfig/Executors/PublicClientExecutor.cs b/src/Microsoft.Identity.Client/ApiConfig/Executors/PublicClientExecutor.cs
@@ -100,7 +100,7 @@ private IWebUI CreateWebAuthenticationDialog(
                 return new CustomWebUiHandler(interactiveParameters.CustomWebUi);
             }
 
-            var coreUiParent = interactiveParameters.UiParent.CoreUiParent;
+            var coreUiParent = interactiveParameters.UiParent;
 
 #if ANDROID || iOS
             coreUiParent.UseEmbeddedWebview = interactiveParameters.UseEmbeddedWebView;
diff --git a/src/Microsoft.Identity.Client/ApiConfig/OwnerUiParent.cs b/src/Microsoft.Identity.Client/ApiConfig/OwnerUiParent.cs
diff --git a/src/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenInteractiveParameters.cs b/src/Microsoft.Identity.Client/ApiConfig/Parameters/AcquireTokenInteractiveParameters.cs
@@ -30,13 +30,14 @@
 using System.Text;
 using Microsoft.Identity.Client.Core;
 using Microsoft.Identity.Client.Extensibility;
+using Microsoft.Identity.Client.UI;
 
 namespace Microsoft.Identity.Client.ApiConfig.Parameters
 {
     internal class AcquireTokenInteractiveParameters : IAcquireTokenParameters
     {
         public Prompt Prompt { get; set; } = Prompt.SelectAccount;
-        public OwnerUiParent UiParent { get; } = new OwnerUiParent();
+        public CoreUIParent UiParent { get; } = new CoreUIParent();
         public IEnumerable<string> ExtraScopesToConsent { get; set; } = new List<string>();
         public bool UseEmbeddedWebView { get; set; }
         public string LoginHint { get; set; }
diff --git a/src/Microsoft.Identity.Client/Internal/Broker/IBroker.cs b/src/Microsoft.Identity.Client/Internal/Broker/IBroker.cs
@@ -25,18 +25,17 @@
 // 
 // ------------------------------------------------------------------------------
 
-using Microsoft.Identity.Client.ApiConfig;
-using Microsoft.Identity.Client.Core;
 using Microsoft.Identity.Client.OAuth2;
+using Microsoft.Identity.Client.UI;
 using System.Collections.Generic;
 using System.Threading.Tasks;
 
 namespace Microsoft.Identity.Client.Internal.Broker
 {
     internal interface IBroker
     {
-        bool CanInvokeBroker(OwnerUiParent uiParent);
+        bool CanInvokeBroker(CoreUIParent uiParent);
 
         Task<MsalTokenResponse> AcquireTokenUsingBrokerAsync(Dictionary<string, string> brokerPayload);
     }
-}
+}
diff --git a/src/Microsoft.Identity.Client/Internal/Broker/NullBroker.cs b/src/Microsoft.Identity.Client/Internal/Broker/NullBroker.cs
@@ -25,10 +25,9 @@
 // 
 // ------------------------------------------------------------------------------
 
-using Microsoft.Identity.Client.ApiConfig;
-using Microsoft.Identity.Client.Core;
 using Microsoft.Identity.Client.Exceptions;
 using Microsoft.Identity.Client.OAuth2;
+using Microsoft.Identity.Client.UI;
 using System;
 using System.Collections.Generic;
 using System.Threading.Tasks;
@@ -40,7 +39,7 @@ namespace Microsoft.Identity.Client.Internal.Broker
     /// </summary>
     internal class NullBroker : IBroker
     {
-        public bool CanInvokeBroker(OwnerUiParent uiParent)
+        public bool CanInvokeBroker(CoreUIParent uiParent)
         {
             return false;
         }
diff --git a/src/Microsoft.Identity.Client/Platforms/iOS/iOSBroker.cs b/src/Microsoft.Identity.Client/Platforms/iOS/iOSBroker.cs
@@ -36,7 +36,7 @@
 using CoreFoundation;
 using Microsoft.Identity.Client.OAuth2;
 using Microsoft.Identity.Client.Internal.Broker;
-using Microsoft.Identity.Client.ApiConfig;
+using Microsoft.Identity.Client.UI;
 
 namespace Microsoft.Identity.Client.Platforms.iOS
 {
@@ -55,15 +55,15 @@ public iOSBroker(IServiceBundle serviceBundle)
             _serviceBundle = serviceBundle;
         }
 
-        public bool CanInvokeBroker(OwnerUiParent uiParent)
+        public bool CanInvokeBroker(CoreUIParent uiParent)
         {
             if (uiParent == null)
             {
                 _serviceBundle.DefaultLogger.Error(iOSBrokerConstants.UiParentIsNullCannotInvokeBroker);
                 return false;
             }
 
-            if (uiParent.CoreUiParent.CallerViewController == null)
+            if (uiParent.CallerViewController == null)
             {
                 _serviceBundle.DefaultLogger.Error(iOSBrokerConstants.CallerViewControllerIsNullCannotInvokeBroker);
                 return false;
@@ -73,7 +73,7 @@ public bool CanInvokeBroker(OwnerUiParent uiParent)
 
             var result = false;
 
-            uiParent.CoreUiParent.CallerViewController.InvokeOnMainThread(() =>
+            uiParent.CallerViewController.InvokeOnMainThread(() =>
             {
                 result = UIApplication.SharedApplication.CanOpenUrl(new NSUrl(BrokerParameter.BrokerV2));
             });
@@ -218,4 +218,4 @@ public static void SetBrokerResponse(NSUrl responseUrl)
             brokerResponseReady.Release();
         }
     }
-}
+}
diff --git a/src/Microsoft.Identity.Client/Platforms/net45/InteractiveWebUI.cs b/src/Microsoft.Identity.Client/Platforms/net45/InteractiveWebUI.cs
@@ -1,4 +1,4 @@
-//----------------------------------------------------------------------
+﻿//----------------------------------------------------------------------
 //
 // Copyright (c) Microsoft Corporation.
 // All rights reserved.
@@ -25,27 +25,32 @@
 //
 //------------------------------------------------------------------------------
 
+using Microsoft.Identity.Client.Core;
 using Microsoft.Identity.Client.UI;
 
 namespace Microsoft.Identity.Client.Platforms.net45
 {
     internal class InteractiveWebUI : WebUI
     {
-#pragma warning disable 618 // WindowsFormsWebAuthenticationDialog is marked obsolete
+        private WindowsFormsWebAuthenticationDialog _dialog;
 
-        private WindowsFormsWebAuthenticationDialog dialog;
+        public InteractiveWebUI(CoreUIParent parent, RequestContext requestContext)
+        {
+            OwnerWindow = parent?.OwnerWindow;
+            SynchronizationContext = parent?.SynchronizationContext;
+            RequestContext = requestContext;
+        }
 
         protected override AuthorizationResult OnAuthenticate()
         {
             AuthorizationResult result;
 
-            using (dialog = new WindowsFormsWebAuthenticationDialog(OwnerWindow) {RequestContext = RequestContext})
+            using (_dialog = new WindowsFormsWebAuthenticationDialog(OwnerWindow) {RequestContext = RequestContext})
             {
-                result = dialog.AuthenticateAAD(RequestUri, CallbackUri);
+                result = _dialog.AuthenticateAAD(RequestUri, CallbackUri);
             }
 
             return result;
         }
-#pragma warning restore 618
     }
-}
+}
diff --git a/src/Microsoft.Identity.Client/Platforms/net45/NetDesktopWebUIFactory.cs b/src/Microsoft.Identity.Client/Platforms/net45/NetDesktopWebUIFactory.cs
@@ -36,10 +36,10 @@ public IWebUI CreateAuthenticationDialog(CoreUIParent parent, RequestContext req
         {
             if (parent.UseHiddenBrowser)
             {
-                return new SilentWebUI {OwnerWindow = parent?.OwnerWindow, RequestContext = requestContext};
+                return new SilentWebUI(parent, requestContext);
             }
 
-            return new InteractiveWebUI {OwnerWindow = parent?.OwnerWindow, RequestContext = requestContext};
+            return new InteractiveWebUI(parent, requestContext);
         }
     }
-}
+}
diff --git a/src/Microsoft.Identity.Client/Platforms/net45/SilentWebUI.cs b/src/Microsoft.Identity.Client/Platforms/net45/SilentWebUI.cs
diff --git a/src/Microsoft.Identity.Client/Platforms/net45/WebUI.cs b/src/Microsoft.Identity.Client/Platforms/net45/WebUI.cs
diff --git a/src/Microsoft.Identity.Client/UI/CoreUIParent.cs b/src/Microsoft.Identity.Client/UI/CoreUIParent.cs

Original file line number	Diff line number	Diff line change
`@@ -74,10 +74,17 @@ internal static AcquireTokenInteractiveParameterBuilder Create(`
`74`	`74`	`object parent)`
`75`	`75`	`{`
`76`	`76`	`return new AcquireTokenInteractiveParameterBuilder(publicClientApplicationExecutor)`
	`77`	`+ .WithCurrentSynchronizationContext()`
`77`	`78`	`.WithScopes(scopes)`
`78`	`79`	`.WithParent(parent);`
`79`	`80`	`}`
`80`	`81`
	`82`	`+ internal AcquireTokenInteractiveParameterBuilder WithCurrentSynchronizationContext()`
	`83`	`+ {`
	`84`	`+ Parameters.UiParent.SynchronizationContext = SynchronizationContext.Current;`
	`85`	`+ return this;`
	`86`	`+ }`
	`87`	`+`
`81`	`88`	`/// <summary>`
`82`	`89`	`/// Specifies if the public client application should used an embedded web browser`
`83`	`90`	`/// or the system default browser`
`@@ -154,7 +161,8 @@ protected override void Validate()`
`154`	`161`	`#if ANDROID`
`155`	`162`	`if (_ownerWindow is Activity activity)`
`156`	`163`	`{`
`157`		`- Parameters.UiParent.SetAndroidActivity(activity);`
	`164`	`+ Parameters.UiParent.Activity = activity;`
	`165`	`+ Parameters.UiParent.CallerActivity = activity;`
`158`	`166`	`}`
`159`	`167`	`else`
`160`	`168`	`{`
`@@ -163,17 +171,17 @@ protected override void Validate()`
`163`	`171`	`#elif iOS`
`164`	`172`	`if(_ownerWindow is UIViewController uiViewController)`
`165`	`173`	`{`
`166`		`- Parameters.UiParent.SetUIViewController(uiViewController);`
	`174`	`+ Parameters.UiParent.CallerViewController = uiViewController;`
`167`	`175`	`}`
`168`	`176`
`169`	`177`	`#elif DESKTOP`
`170`	`178`	`if (_ownerWindow is IWin32Window win32Window)`
`171`	`179`	`{`
`172`		`- Parameters.UiParent.SetOwnerWindow(win32Window);`
	`180`	`+ Parameters.UiParent.OwnerWindow = win32Window;`
`173`	`181`	`}`
`174`	`182`	`else if (_ownerWindow is IntPtr intPtrWindow)`
`175`	`183`	`{`
`176`		`- Parameters.UiParent.SetOwnerWindow(intPtrWindow);`
	`184`	`+ Parameters.UiParent.OwnerWindow = intPtrWindow;`
`177`	`185`	`}`
`178`	`186`	`// It's ok on Windows Desktop to not have an owner window, the system will just center on the display`
`179`	`187`	`// instead of a parent.`
Original file line number	Diff line number	Diff line change
`@@ -100,7 +100,7 @@ private IWebUI CreateWebAuthenticationDialog(`
`100`	`100`	`return new CustomWebUiHandler(interactiveParameters.CustomWebUi);`
`101`	`101`	`}`
`102`	`102`
`103`		`- var coreUiParent = interactiveParameters.UiParent.CoreUiParent;`
	`103`	`+ var coreUiParent = interactiveParameters.UiParent;`
`104`	`104`
`105`	`105`	`#if ANDROID \|\| iOS`
`106`	`106`	`coreUiParent.UseEmbeddedWebview = interactiveParameters.UseEmbeddedWebView;`
Original file line number	Diff line number	Diff line change
`@@ -25,18 +25,17 @@`
`25`	`25`	`//`
`26`	`26`	`// ------------------------------------------------------------------------------`
`27`	`27`
`28`		`-using Microsoft.Identity.Client.ApiConfig;`
`29`		`-using Microsoft.Identity.Client.Core;`
`30`	`28`	`using Microsoft.Identity.Client.OAuth2;`
	`29`	`+using Microsoft.Identity.Client.UI;`
`31`	`30`	`using System.Collections.Generic;`
`32`	`31`	`using System.Threading.Tasks;`
`33`	`32`
`34`	`33`	`namespace Microsoft.Identity.Client.Internal.Broker`
`35`	`34`	`{`
`36`	`35`	`internal interface IBroker`
`37`	`36`	`{`
`38`		`- bool CanInvokeBroker(OwnerUiParent uiParent);`
	`37`	`+ bool CanInvokeBroker(CoreUIParent uiParent);`
`39`	`38`
`40`	`39`	`Task<MsalTokenResponse> AcquireTokenUsingBrokerAsync(Dictionary<string, string> brokerPayload);`
`41`	`40`	`}`
`42`		`-}`
	`41`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -25,10 +25,9 @@`
`25`	`25`	`//`
`26`	`26`	`// ------------------------------------------------------------------------------`
`27`	`27`
`28`		`-using Microsoft.Identity.Client.ApiConfig;`
`29`		`-using Microsoft.Identity.Client.Core;`
`30`	`28`	`using Microsoft.Identity.Client.Exceptions;`
`31`	`29`	`using Microsoft.Identity.Client.OAuth2;`
	`30`	`+using Microsoft.Identity.Client.UI;`
`32`	`31`	`using System;`
`33`	`32`	`using System.Collections.Generic;`
`34`	`33`	`using System.Threading.Tasks;`
`@@ -40,7 +39,7 @@ namespace Microsoft.Identity.Client.Internal.Broker`
`40`	`39`	`/// </summary>`
`41`	`40`	`internal class NullBroker : IBroker`
`42`	`41`	`{`
`43`		`- public bool CanInvokeBroker(OwnerUiParent uiParent)`
	`42`	`+ public bool CanInvokeBroker(CoreUIParent uiParent)`
`44`	`43`	`{`
`45`	`44`	`return false;`
`46`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@`
`36`	`36`	`using CoreFoundation;`
`37`	`37`	`using Microsoft.Identity.Client.OAuth2;`
`38`	`38`	`using Microsoft.Identity.Client.Internal.Broker;`
`39`		`-using Microsoft.Identity.Client.ApiConfig;`
	`39`	`+using Microsoft.Identity.Client.UI;`
`40`	`40`
`41`	`41`	`namespace Microsoft.Identity.Client.Platforms.iOS`
`42`	`42`	`{`
`@@ -55,15 +55,15 @@ public iOSBroker(IServiceBundle serviceBundle)`
`55`	`55`	`_serviceBundle = serviceBundle;`
`56`	`56`	`}`
`57`	`57`
`58`		`- public bool CanInvokeBroker(OwnerUiParent uiParent)`
	`58`	`+ public bool CanInvokeBroker(CoreUIParent uiParent)`
`59`	`59`	`{`
`60`	`60`	`if (uiParent == null)`
`61`	`61`	`{`
`62`	`62`	`_serviceBundle.DefaultLogger.Error(iOSBrokerConstants.UiParentIsNullCannotInvokeBroker);`
`63`	`63`	`return false;`
`64`	`64`	`}`
`65`	`65`
`66`		`- if (uiParent.CoreUiParent.CallerViewController == null)`
	`66`	`+ if (uiParent.CallerViewController == null)`
`67`	`67`	`{`
`68`	`68`	`_serviceBundle.DefaultLogger.Error(iOSBrokerConstants.CallerViewControllerIsNullCannotInvokeBroker);`
`69`	`69`	`return false;`
`@@ -73,7 +73,7 @@ public bool CanInvokeBroker(OwnerUiParent uiParent)`
`73`	`73`
`74`	`74`	`var result = false;`
`75`	`75`
`76`		`- uiParent.CoreUiParent.CallerViewController.InvokeOnMainThread(() =>`
	`76`	`+ uiParent.CallerViewController.InvokeOnMainThread(() =>`
`77`	`77`	`{`
`78`	`78`	`result = UIApplication.SharedApplication.CanOpenUrl(new NSUrl(BrokerParameter.BrokerV2));`
`79`	`79`	`});`
`@@ -218,4 +218,4 @@ public static void SetBrokerResponse(NSUrl responseUrl)`
`218`	`218`	`brokerResponseReady.Release();`
`219`	`219`	`}`
`220`	`220`	`}`
`221`		`-}`
	`221`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-//----------------------------------------------------------------------`
	`1`	`+//----------------------------------------------------------------------`
`2`	`2`	`//`
`3`	`3`	`// Copyright (c) Microsoft Corporation.`
`4`	`4`	`// All rights reserved.`
`@@ -25,27 +25,32 @@`
`25`	`25`	`//`
`26`	`26`	`//------------------------------------------------------------------------------`
`27`	`27`
	`28`	`+using Microsoft.Identity.Client.Core;`
`28`	`29`	`using Microsoft.Identity.Client.UI;`
`29`	`30`
`30`	`31`	`namespace Microsoft.Identity.Client.Platforms.net45`
`31`	`32`	`{`
`32`	`33`	`internal class InteractiveWebUI : WebUI`
`33`	`34`	`{`
`34`		`-#pragma warning disable 618 // WindowsFormsWebAuthenticationDialog is marked obsolete`
	`35`	`+ private WindowsFormsWebAuthenticationDialog _dialog;`
`35`	`36`
`36`		`- private WindowsFormsWebAuthenticationDialog dialog;`
	`37`	`+ public InteractiveWebUI(CoreUIParent parent, RequestContext requestContext)`
	`38`	`+ {`
	`39`	`+ OwnerWindow = parent?.OwnerWindow;`
	`40`	`+ SynchronizationContext = parent?.SynchronizationContext;`
	`41`	`+ RequestContext = requestContext;`
	`42`	`+ }`
`37`	`43`
`38`	`44`	`protected override AuthorizationResult OnAuthenticate()`
`39`	`45`	`{`
`40`	`46`	`AuthorizationResult result;`
`41`	`47`
`42`		`- using (dialog = new WindowsFormsWebAuthenticationDialog(OwnerWindow) {RequestContext = RequestContext})`
	`48`	`+ using (_dialog = new WindowsFormsWebAuthenticationDialog(OwnerWindow) {RequestContext = RequestContext})`
`43`	`49`	`{`
`44`		`- result = dialog.AuthenticateAAD(RequestUri, CallbackUri);`
	`50`	`+ result = _dialog.AuthenticateAAD(RequestUri, CallbackUri);`
`45`	`51`	`}`
`46`	`52`
`47`	`53`	`return result;`
`48`	`54`	`}`
`49`		`-#pragma warning restore 618`
`50`	`55`	`}`
`51`		`-}`
	`56`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -36,10 +36,10 @@ public IWebUI CreateAuthenticationDialog(CoreUIParent parent, RequestContext req`
`36`	`36`	`{`
`37`	`37`	`if (parent.UseHiddenBrowser)`
`38`	`38`	`{`
`39`		`- return new SilentWebUI {OwnerWindow = parent?.OwnerWindow, RequestContext = requestContext};`
	`39`	`+ return new SilentWebUI(parent, requestContext);`
`40`	`40`	`}`
`41`	`41`
`42`		`- return new InteractiveWebUI {OwnerWindow = parent?.OwnerWindow, RequestContext = requestContext};`
	`42`	`+ return new InteractiveWebUI(parent, requestContext);`
`43`	`43`	`}`
`44`	`44`	`}`
`45`		`-}`
	`45`	`+}`