From 92568588608e2ae5fbd2409b828b130e75f4a026 Mon Sep 17 00:00:00 2001 From: Shane Weaver Date: Mon, 13 Dec 2021 11:54:15 -0800 Subject: [PATCH] Added logging to auth providers --- .../MsalProvider.cs | 35 ++++++++++--- .../WindowsProvider.cs | 51 +++++++++---------- .../BaseProvider.cs | 6 +++ .../Logging/DebugLogger.cs | 25 +++++++++ .../Logging/ILogger.cs | 18 +++++++ 5 files changed, 100 insertions(+), 35 deletions(-) create mode 100644 CommunityToolkit.Authentication/Logging/DebugLogger.cs create mode 100644 CommunityToolkit.Authentication/Logging/ILogger.cs diff --git a/CommunityToolkit.Authentication.Msal/MsalProvider.cs b/CommunityToolkit.Authentication.Msal/MsalProvider.cs index 9f41e58..a30812d 100644 --- a/CommunityToolkit.Authentication.Msal/MsalProvider.cs +++ b/CommunityToolkit.Authentication.Msal/MsalProvider.cs @@ -80,9 +80,10 @@ public MsalProvider(IPublicClientApplication client, string[] scopes = null, boo /// Determines whether the provider attempts to silently log in upon creation. /// Determines if organizational accounts should be enabled/disabled. /// Registered tenant id in Azure Active Directory. - public MsalProvider(string clientId, string[] scopes = null, string redirectUri = null, bool autoSignIn = true, bool listWindowsWorkAndSchoolAccounts = true, string tenantId = null) + /// Output logs. + public MsalProvider(string clientId, string[] scopes = null, string redirectUri = null, bool autoSignIn = true, bool listWindowsWorkAndSchoolAccounts = true, string tenantId = null, bool withLogging = false) { - Client = CreatePublicClientApplication(clientId, tenantId, redirectUri, listWindowsWorkAndSchoolAccounts); + Client = CreatePublicClientApplication(clientId, tenantId, redirectUri, listWindowsWorkAndSchoolAccounts, withLogging); Scopes = scopes.Select(s => s.ToLower()).ToArray() ?? new string[] { string.Empty }; if (autoSignIn) @@ -183,8 +184,9 @@ public override Task GetTokenAsync(bool silentOnly = false) /// An optional tenant id. /// Redirect uri for auth response. /// Determines if organizational accounts should be supported. + /// Output logs. /// A new instance of . - protected IPublicClientApplication CreatePublicClientApplication(string clientId, string tenantId, string redirectUri, bool listWindowsWorkAndSchoolAccounts) + protected IPublicClientApplication CreatePublicClientApplication(string clientId, string tenantId, string redirectUri, bool listWindowsWorkAndSchoolAccounts, bool withLogging) { var authority = listWindowsWorkAndSchoolAccounts ? AadAuthorityAudience.AzureAdAndPersonalMicrosoftAccount : AadAuthorityAudience.PersonalMicrosoftAccount; @@ -193,6 +195,20 @@ protected IPublicClientApplication CreatePublicClientApplication(string clientId .WithClientName(ProviderManager.ClientName) .WithClientVersion(Assembly.GetExecutingAssembly().GetName().Version.ToString()); + if (withLogging) + { + clientBuilder = clientBuilder.WithLogging((level, message, containsPii) => + { + if (containsPii) + { + // Add a PII warning to messages containing any. + message = $"[CONTAINS PII] {message}"; + } + + EventLogger.Log($"{level}: {message}"); + }); + } + if (tenantId != null) { clientBuilder = clientBuilder.WithTenantId(tenantId); @@ -232,13 +248,15 @@ protected async Task GetTokenWithScopesAsync(string[] scopes, bool silen authResult = await Client.AcquireTokenSilent(scopes, account).ExecuteAsync(); } } - catch (MsalUiRequiredException) + catch (MsalUiRequiredException e) { + EventLogger.Log(e.Message); } - catch + catch (Exception e) { // Unexpected exception - // TODO: Send exception to a logger. + EventLogger.Log(e.Message); + EventLogger.Log(e.StackTrace); } if (authResult == null && !silentOnly) @@ -266,10 +284,11 @@ protected async Task GetTokenWithScopesAsync(string[] scopes, bool silen authResult = await paramBuilder.ExecuteAsync(); } - catch + catch (Exception e) { // Unexpected exception - // TODO: Send exception to a logger. + EventLogger.Log(e.Message); + EventLogger.Log(e.StackTrace); } } diff --git a/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs b/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs index b0f1235..e2bf5b4 100644 --- a/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs +++ b/CommunityToolkit.Authentication.Uwp/WindowsProvider.cs @@ -172,9 +172,11 @@ public override async Task SignOutAsync() { await _webAccount.SignOutAsync(); } - catch + catch (Exception e) { // Failed to remove an account. + EventLogger.Log(e.Message); + EventLogger.Log(e.StackTrace); } _webAccount = null; @@ -232,7 +234,8 @@ public override async Task GetTokenAsync(bool silentOnly = false) } catch (Exception e) { - // TODO: Log failure + EventLogger.Log(e.Message); + EventLogger.Log(e.StackTrace); throw e; } finally @@ -363,36 +366,29 @@ private async Task SetAccountAsync(WebAccount account) private async Task AuthenticateSilentAsync(string[] scopes) { - try - { - WebTokenRequestResult authResult = null; + WebTokenRequestResult authResult = null; - var account = _webAccount; - if (account == null) - { - // Check the cache for an existing user - if (Settings[SettingsKeyAccountId] is string savedAccountId && - Settings[SettingsKeyProviderId] is string savedProviderId && - Settings[SettingsKeyProviderAuthority] is string savedProviderAuthority) - { - var savedProvider = await WebAuthenticationCoreManager.FindAccountProviderAsync(savedProviderId, savedProviderAuthority); - account = await WebAuthenticationCoreManager.FindAccountAsync(savedProvider, savedAccountId); - } - } - - if (account != null) + var account = _webAccount; + if (account == null) + { + // Check the cache for an existing user + if (Settings[SettingsKeyAccountId] is string savedAccountId && + Settings[SettingsKeyProviderId] is string savedProviderId && + Settings[SettingsKeyProviderAuthority] is string savedProviderAuthority) { - // Prepare a request to get a token. - var webTokenRequest = GetWebTokenRequest(account.WebAccountProvider, _webAccountProviderConfig.ClientId, scopes); - authResult = await WebAuthenticationCoreManager.GetTokenSilentlyAsync(webTokenRequest, account); + var savedProvider = await WebAuthenticationCoreManager.FindAccountProviderAsync(savedProviderId, savedProviderAuthority); + account = await WebAuthenticationCoreManager.FindAccountAsync(savedProvider, savedAccountId); } - - return authResult; } - catch (HttpRequestException) + + if (account != null) { - throw; /* probably offline, no point continuing to interactive auth */ + // Prepare a request to get a token. + var webTokenRequest = GetWebTokenRequest(account.WebAccountProvider, _webAccountProviderConfig.ClientId, scopes); + authResult = await WebAuthenticationCoreManager.GetTokenSilentlyAsync(webTokenRequest, account); } + + return authResult; } private Task AuthenticateInteractiveAsync(string[] scopes) @@ -527,9 +523,10 @@ async void OnAccountCommandsRequested(AccountsSettingsPane sender, AccountsSetti var webAccountProvider = webAccountProviderCommandWasInvoked ? await webAccountProviderTaskCompletionSource.Task : null; return webAccountProvider; } - catch (TaskCanceledException) + catch (TaskCanceledException e) { // The task was cancelled. No provider was chosen. + EventLogger.Log(e.Message); return null; } finally diff --git a/CommunityToolkit.Authentication/BaseProvider.cs b/CommunityToolkit.Authentication/BaseProvider.cs index cbeaeb6..17a8cee 100644 --- a/CommunityToolkit.Authentication/BaseProvider.cs +++ b/CommunityToolkit.Authentication/BaseProvider.cs @@ -6,6 +6,7 @@ using System.Net.Http; using System.Threading.Tasks; using CommunityToolkit.Authentication.Extensions; +using CommunityToolkit.Authentication.Logging; namespace CommunityToolkit.Authentication { @@ -34,6 +35,11 @@ protected set } } + /// + /// Gets or sets the logger used to log exceptions and event messages. + /// + public ILogger EventLogger { get; set; } = DebugLogger.Instance; + /// public abstract string CurrentAccountId { get; } diff --git a/CommunityToolkit.Authentication/Logging/DebugLogger.cs b/CommunityToolkit.Authentication/Logging/DebugLogger.cs new file mode 100644 index 0000000..8f0b515 --- /dev/null +++ b/CommunityToolkit.Authentication/Logging/DebugLogger.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Diagnostics; + +namespace CommunityToolkit.Authentication.Logging +{ + /// + /// Logs messages using the API. + /// + public class DebugLogger : ILogger + { + /// + /// Singleton instance. + /// + public static readonly DebugLogger Instance = new DebugLogger(); + + /// + public void Log(string message) + { + Debug.WriteLine(message); + } + } +} diff --git a/CommunityToolkit.Authentication/Logging/ILogger.cs b/CommunityToolkit.Authentication/Logging/ILogger.cs new file mode 100644 index 0000000..47d7ac8 --- /dev/null +++ b/CommunityToolkit.Authentication/Logging/ILogger.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +namespace CommunityToolkit.Authentication.Logging +{ + /// + /// Defines a simple logger for handling messages during runtime. + /// + public interface ILogger + { + /// + /// Log a message. + /// + /// The message to log. + void Log(string message); + } +}