From 06d0c6d47b47485ef7af34400f16138b58c41aa2 Mon Sep 17 00:00:00 2001 From: Shane Weaver Date: Wed, 18 Aug 2021 16:59:27 -0700 Subject: [PATCH 1/4] Add support for incremental consent via MsalProvider --- ...ommunityToolkit.Authentication.Msal.csproj | 1 + .../MsalProvider.cs | 85 +++++++++++++------ .../WindowsProvider.cs | 10 ++- .../BaseProvider.cs | 2 +- CommunityToolkit.Authentication/IProvider.cs | 3 +- .../MockProvider.cs | 2 +- 6 files changed, 73 insertions(+), 30 deletions(-) diff --git a/CommunityToolkit.Authentication.Msal/CommunityToolkit.Authentication.Msal.csproj b/CommunityToolkit.Authentication.Msal/CommunityToolkit.Authentication.Msal.csproj index 050e3e8..7bfb3bd 100644 --- a/CommunityToolkit.Authentication.Msal/CommunityToolkit.Authentication.Msal.csproj +++ b/CommunityToolkit.Authentication.Msal/CommunityToolkit.Authentication.Msal.csproj @@ -14,6 +14,7 @@ + diff --git a/CommunityToolkit.Authentication.Msal/MsalProvider.cs b/CommunityToolkit.Authentication.Msal/MsalProvider.cs index b871901..285d913 100644 --- a/CommunityToolkit.Authentication.Msal/MsalProvider.cs +++ b/CommunityToolkit.Authentication.Msal/MsalProvider.cs @@ -6,7 +6,9 @@ using System.Net.Http; using System.Net.Http.Headers; using System.Reflection; +using System.Threading; using System.Threading.Tasks; +using Microsoft.Graph; using Microsoft.Identity.Client; namespace CommunityToolkit.Authentication @@ -16,6 +18,8 @@ namespace CommunityToolkit.Authentication /// public class MsalProvider : BaseProvider { + private static readonly SemaphoreSlim SemaphoreSlim = new (1); + /// public override string CurrentAccountId => _account?.HomeAccountId?.Identifier; @@ -47,7 +51,7 @@ public MsalProvider(string clientId, string[] scopes = null, string redirectUri .WithClientVersion(Assembly.GetExecutingAssembly().GetName().Version.ToString()) .Build(); - Scopes = scopes ?? new string[] { string.Empty }; + Scopes = scopes.Select(s => s.ToLower()).ToArray() ?? new string[] { string.Empty }; Client = client; @@ -60,7 +64,22 @@ public MsalProvider(string clientId, string[] scopes = null, string redirectUri /// public override async Task AuthenticateRequestAsync(HttpRequestMessage request) { - string token = await GetTokenAsync(); + string token; + + // Check if any specific scopes are being requested. + if (request.Properties.TryGetValue(nameof(GraphRequestContext), out object requestContextObj) && + requestContextObj is GraphRequestContext requestContext && + requestContext.MiddlewareOptions.TryGetValue(nameof(AuthenticationHandlerOption), out IMiddlewareOption optionsMiddleware) && + optionsMiddleware is AuthenticationHandlerOption options) + { + var withScopes = options.AuthenticationProviderOption.Scopes; + token = await this.GetTokenAsync(false, withScopes); + } + else + { + token = await this.GetTokenAsync(); + } + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); } @@ -119,42 +138,60 @@ public override async Task SignOutAsync() } /// - public override async Task GetTokenAsync(bool silentOnly = false) + public override async Task GetTokenAsync(bool silentOnly = false, string[] withScopes = null) { - AuthenticationResult authResult = null; + await SemaphoreSlim.WaitAsync(); + try { - var account = _account ?? (await Client.GetAccountsAsync()).FirstOrDefault(); - if (account != null) - { - authResult = await Client.AcquireTokenSilent(Scopes, account).ExecuteAsync(); - } - } - catch (MsalUiRequiredException) - { - } - catch - { - // Unexpected exception - // TODO: Send exception to a logger. - } + var scopes = withScopes ?? Scopes; - if (authResult == null && !silentOnly) - { + AuthenticationResult authResult = null; try { - authResult = await Client.AcquireTokenInteractive(Scopes).WithPrompt(Prompt.SelectAccount).ExecuteAsync(); + var account = _account ?? (await Client.GetAccountsAsync()).FirstOrDefault(); + if (account != null) + { + authResult = await Client.AcquireTokenSilent(scopes, account).ExecuteAsync(); + } + } + catch (MsalUiRequiredException) + { } catch { // Unexpected exception // TODO: Send exception to a logger. } - } - _account = authResult?.Account; + if (authResult == null && !silentOnly) + { + try + { + if (_account != null) + { + authResult = await Client.AcquireTokenInteractive(scopes).WithPrompt(Prompt.NoPrompt).WithAccount(_account).ExecuteAsync(); + } + else + { + authResult = await Client.AcquireTokenInteractive(scopes).WithPrompt(Prompt.NoPrompt).ExecuteAsync(); + } + } + catch + { + // Unexpected exception + // TODO: Send exception to a logger. + } + } + + _account = authResult?.Account; - return authResult?.AccessToken; + return authResult?.AccessToken; + } + finally + { + SemaphoreSlim.Release(); + } } } } diff --git a/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs b/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs index a7864c8..60f7930 100644 --- a/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs +++ b/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs @@ -179,7 +179,7 @@ public override async Task SignOutAsync() } /// - public override async Task GetTokenAsync(bool silentOnly = false) + public override async Task GetTokenAsync(bool silentOnly = false, string[] withScopes = null) { var internetConnectionProfile = NetworkInformation.GetInternetConnectionProfile(); if (internetConnectionProfile == null) @@ -189,10 +189,14 @@ public override async Task GetTokenAsync(bool silentOnly = false) return null; } + // WebAuthenticationCoreManager does not support incremental consent so we ignore the withScopes param. + // var scopes = withScopes ?? _scopes; + var scopes = _scopes; + try { // Attempt to authenticate silently. - var authResult = await AuthenticateSilentAsync(_scopes); + var authResult = await AuthenticateSilentAsync(scopes); // Authenticate with user interaction as appropriate. if (authResult?.ResponseStatus != WebTokenRequestStatus.Success) @@ -204,7 +208,7 @@ public override async Task GetTokenAsync(bool silentOnly = false) } // Attempt to authenticate interactively. - authResult = await AuthenticateInteractiveAsync(_scopes); + authResult = await AuthenticateInteractiveAsync(scopes); } if (authResult?.ResponseStatus == WebTokenRequestStatus.Success) diff --git a/CommunityToolkit.Authentication/BaseProvider.cs b/CommunityToolkit.Authentication/BaseProvider.cs index cbeaeb6..149355c 100644 --- a/CommunityToolkit.Authentication/BaseProvider.cs +++ b/CommunityToolkit.Authentication/BaseProvider.cs @@ -52,7 +52,7 @@ public BaseProvider() public abstract Task AuthenticateRequestAsync(HttpRequestMessage request); /// - public abstract Task GetTokenAsync(bool silentOnly = false); + public abstract Task GetTokenAsync(bool silentOnly = false, string[] withScopes = null); /// public abstract Task SignInAsync(); diff --git a/CommunityToolkit.Authentication/IProvider.cs b/CommunityToolkit.Authentication/IProvider.cs index cd065a2..b50f13c 100644 --- a/CommunityToolkit.Authentication/IProvider.cs +++ b/CommunityToolkit.Authentication/IProvider.cs @@ -39,8 +39,9 @@ public interface IProvider /// Retrieve a token for the authenticated user. /// /// Determines if the acquisition should be done without prompts to the user. + /// Additional scopes to request access for. Only for use with auth providers that support incremental consent. /// A token string for the authenticated user. - Task GetTokenAsync(bool silentOnly = false); + Task GetTokenAsync(bool silentOnly = false, string[] withScopes = null); /// /// Sign in the user. diff --git a/CommunityToolkit.Authentication/MockProvider.cs b/CommunityToolkit.Authentication/MockProvider.cs index d66e63b..88b21f8 100644 --- a/CommunityToolkit.Authentication/MockProvider.cs +++ b/CommunityToolkit.Authentication/MockProvider.cs @@ -52,7 +52,7 @@ public override async Task AuthenticateRequestAsync(HttpRequestMessage request) } /// - public override Task GetTokenAsync(bool silentOnly = false) + public override Task GetTokenAsync(bool silentOnly = false, string[] withScopes = null) { return Task.FromResult(""); } From c6b7046a5a139dcf53e7cc93acf10dc7a81fb07c Mon Sep 17 00:00:00 2001 From: Shane Weaver Date: Wed, 18 Aug 2021 17:13:26 -0700 Subject: [PATCH 2/4] Reverting changes to IProvider --- CommunityToolkit.Authentication.Msal/MsalProvider.cs | 11 +++++++---- .../WindowsProvider.cs | 4 +--- CommunityToolkit.Authentication/BaseProvider.cs | 2 +- CommunityToolkit.Authentication/IProvider.cs | 3 +-- CommunityToolkit.Authentication/MockProvider.cs | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/CommunityToolkit.Authentication.Msal/MsalProvider.cs b/CommunityToolkit.Authentication.Msal/MsalProvider.cs index 285d913..930cfa2 100644 --- a/CommunityToolkit.Authentication.Msal/MsalProvider.cs +++ b/CommunityToolkit.Authentication.Msal/MsalProvider.cs @@ -73,7 +73,7 @@ requestContextObj is GraphRequestContext requestContext && optionsMiddleware is AuthenticationHandlerOption options) { var withScopes = options.AuthenticationProviderOption.Scopes; - token = await this.GetTokenAsync(false, withScopes); + token = await this.GetTokenWithScopesAsync(withScopes); } else { @@ -138,14 +138,17 @@ public override async Task SignOutAsync() } /// - public override async Task GetTokenAsync(bool silentOnly = false, string[] withScopes = null) + public override Task GetTokenAsync(bool silentOnly = false) + { + return GetTokenWithScopesAsync(Scopes, silentOnly); + } + + private async Task GetTokenWithScopesAsync(string[] scopes, bool silentOnly = false) { await SemaphoreSlim.WaitAsync(); try { - var scopes = withScopes ?? Scopes; - AuthenticationResult authResult = null; try { diff --git a/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs b/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs index 60f7930..b26cec3 100644 --- a/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs +++ b/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs @@ -179,7 +179,7 @@ public override async Task SignOutAsync() } /// - public override async Task GetTokenAsync(bool silentOnly = false, string[] withScopes = null) + public override async Task GetTokenAsync(bool silentOnly = false) { var internetConnectionProfile = NetworkInformation.GetInternetConnectionProfile(); if (internetConnectionProfile == null) @@ -189,8 +189,6 @@ public override async Task GetTokenAsync(bool silentOnly = false, string return null; } - // WebAuthenticationCoreManager does not support incremental consent so we ignore the withScopes param. - // var scopes = withScopes ?? _scopes; var scopes = _scopes; try diff --git a/CommunityToolkit.Authentication/BaseProvider.cs b/CommunityToolkit.Authentication/BaseProvider.cs index 149355c..cbeaeb6 100644 --- a/CommunityToolkit.Authentication/BaseProvider.cs +++ b/CommunityToolkit.Authentication/BaseProvider.cs @@ -52,7 +52,7 @@ public BaseProvider() public abstract Task AuthenticateRequestAsync(HttpRequestMessage request); /// - public abstract Task GetTokenAsync(bool silentOnly = false, string[] withScopes = null); + public abstract Task GetTokenAsync(bool silentOnly = false); /// public abstract Task SignInAsync(); diff --git a/CommunityToolkit.Authentication/IProvider.cs b/CommunityToolkit.Authentication/IProvider.cs index b50f13c..cd065a2 100644 --- a/CommunityToolkit.Authentication/IProvider.cs +++ b/CommunityToolkit.Authentication/IProvider.cs @@ -39,9 +39,8 @@ public interface IProvider /// Retrieve a token for the authenticated user. /// /// Determines if the acquisition should be done without prompts to the user. - /// Additional scopes to request access for. Only for use with auth providers that support incremental consent. /// A token string for the authenticated user. - Task GetTokenAsync(bool silentOnly = false, string[] withScopes = null); + Task GetTokenAsync(bool silentOnly = false); /// /// Sign in the user. diff --git a/CommunityToolkit.Authentication/MockProvider.cs b/CommunityToolkit.Authentication/MockProvider.cs index 88b21f8..d66e63b 100644 --- a/CommunityToolkit.Authentication/MockProvider.cs +++ b/CommunityToolkit.Authentication/MockProvider.cs @@ -52,7 +52,7 @@ public override async Task AuthenticateRequestAsync(HttpRequestMessage request) } /// - public override Task GetTokenAsync(bool silentOnly = false, string[] withScopes = null) + public override Task GetTokenAsync(bool silentOnly = false) { return Task.FromResult(""); } From d003a6c0a9c8e0ed855b126bb41cc49259eceb54 Mon Sep 17 00:00:00 2001 From: Shane Weaver Date: Wed, 18 Aug 2021 17:33:00 -0700 Subject: [PATCH 3/4] Improved safety when checking for alternative scopes --- CommunityToolkit.Authentication.Msal/MsalProvider.cs | 7 ++++--- .../Controls/PersonView/PersonView.cs | 5 +++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/CommunityToolkit.Authentication.Msal/MsalProvider.cs b/CommunityToolkit.Authentication.Msal/MsalProvider.cs index 930cfa2..419abdb 100644 --- a/CommunityToolkit.Authentication.Msal/MsalProvider.cs +++ b/CommunityToolkit.Authentication.Msal/MsalProvider.cs @@ -69,8 +69,9 @@ public override async Task AuthenticateRequestAsync(HttpRequestMessage request) // Check if any specific scopes are being requested. if (request.Properties.TryGetValue(nameof(GraphRequestContext), out object requestContextObj) && requestContextObj is GraphRequestContext requestContext && - requestContext.MiddlewareOptions.TryGetValue(nameof(AuthenticationHandlerOption), out IMiddlewareOption optionsMiddleware) && - optionsMiddleware is AuthenticationHandlerOption options) + requestContext.MiddlewareOptions.TryGetValue(nameof(AuthenticationHandlerOption), out IMiddlewareOption optionsMiddleware) && + optionsMiddleware is AuthenticationHandlerOption options && + options.AuthenticationProviderOption?.Scopes != null && options.AuthenticationProviderOption.Scopes.Length > 0) { var withScopes = options.AuthenticationProviderOption.Scopes; token = await this.GetTokenWithScopesAsync(withScopes); @@ -140,7 +141,7 @@ public override async Task SignOutAsync() /// public override Task GetTokenAsync(bool silentOnly = false) { - return GetTokenWithScopesAsync(Scopes, silentOnly); + return this.GetTokenWithScopesAsync(Scopes, silentOnly); } private async Task GetTokenWithScopesAsync(string[] scopes, bool silentOnly = false) diff --git a/CommunityToolkit.Graph.Uwp/Controls/PersonView/PersonView.cs b/CommunityToolkit.Graph.Uwp/Controls/PersonView/PersonView.cs index 8771965..5f3deee 100644 --- a/CommunityToolkit.Graph.Uwp/Controls/PersonView/PersonView.cs +++ b/CommunityToolkit.Graph.Uwp/Controls/PersonView/PersonView.cs @@ -131,6 +131,11 @@ private async void LoadData() else { LoadDefaultImage(); + + if (!string.IsNullOrWhiteSpace(UserId) || !string.IsNullOrWhiteSpace(PersonQuery)) + { + PersonDetails = null; + } } } From d5360086df7684c719419f52846dcbfa97409a11 Mon Sep 17 00:00:00 2001 From: Shane Weaver Date: Wed, 18 Aug 2021 17:39:59 -0700 Subject: [PATCH 4/4] Added SemaphoreSlim to WindowsProvider GetTokenAsync --- .../WindowsProvider.cs | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs b/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs index b26cec3..223a443 100644 --- a/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs +++ b/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Net.Http; using System.Net.Http.Headers; +using System.Threading; using System.Threading.Tasks; using Windows.Networking.Connectivity; using Windows.Security.Authentication.Web; @@ -28,6 +29,8 @@ public class WindowsProvider : BaseProvider /// public static string RedirectUri => string.Format("ms-appx-web://Microsoft.AAD.BrokerPlugIn/{0}", WebAuthenticationBroker.GetCurrentApplicationCallbackUri().Host.ToUpper()); + private static readonly SemaphoreSlim SemaphoreSlim = new(1); + private const string AuthenticationHeaderScheme = "Bearer"; private const string GraphResourcePropertyKey = "resource"; private const string GraphResourcePropertyValue = "https://graph.microsoft.com"; @@ -181,18 +184,20 @@ public override async Task SignOutAsync() /// public override async Task GetTokenAsync(bool silentOnly = false) { - var internetConnectionProfile = NetworkInformation.GetInternetConnectionProfile(); - if (internetConnectionProfile == null) - { - // We are not online, no token for you. - // TODO: Is there anything special to do when we go offline? - return null; - } - - var scopes = _scopes; + await SemaphoreSlim.WaitAsync(); try { + var internetConnectionProfile = NetworkInformation.GetInternetConnectionProfile(); + if (internetConnectionProfile == null) + { + // We are not online, no token for you. + // TODO: Is there anything special to do when we go offline? + return null; + } + + var scopes = _scopes; + // Attempt to authenticate silently. var authResult = await AuthenticateSilentAsync(scopes); @@ -234,9 +239,12 @@ public override async Task GetTokenAsync(bool silentOnly = false) catch (Exception e) { // TODO: Log failure - System.Diagnostics.Debug.WriteLine(e.Message); throw e; } + finally + { + SemaphoreSlim.Release(); + } } ///