From b4f7bd01af77fa6d46ecf8b84d3ba51b4abed07e Mon Sep 17 00:00:00 2001 From: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com> Date: Tue, 30 May 2023 14:46:18 -0700 Subject: [PATCH] Fix email extraction from username (#2075) --- .../Authenticator.cs | 73 +++++++++++++------ .../Authentication/AuthenticatorTests.cs | 50 +++++++++++++ 2 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 test/Microsoft.SqlTools.ServiceLayer.UnitTests/Authentication/AuthenticatorTests.cs diff --git a/src/Microsoft.SqlTools.Authentication/Authenticator.cs b/src/Microsoft.SqlTools.Authentication/Authenticator.cs index e3f279a0..d6b30531 100644 --- a/src/Microsoft.SqlTools.Authentication/Authenticator.cs +++ b/src/Microsoft.SqlTools.Authentication/Authenticator.cs @@ -13,7 +13,7 @@ namespace Microsoft.SqlTools.Authentication /// /// Provides APIs to acquire access token using MSAL.NET v4 with provided . /// - public class Authenticator: IAuthenticator + public class Authenticator : IAuthenticator { private AuthenticatorConfiguration configuration; @@ -58,10 +58,27 @@ namespace Microsoft.SqlTools.Authentication IEnumerator? accounts = (await publicClientApplication.GetAccountsAsync().ConfigureAwait(false)).GetEnumerator(); IAccount? account = default; - if (!string.IsNullOrEmpty(@params.UserName) && accounts.MoveNext()) + if (!string.IsNullOrEmpty(@params.UserName)) { - // Handle username format to extract email: "John Doe - johndoe@constoso.com" - string username = @params.UserName.Contains(" - ") ? @params.UserName.Split(" - ")[1] : @params.UserName; + // Handle username format to extract email: "John Doe - johndoe@constoso.com" as received from ADS/VSCode-MSSQL + + // Additional possible usernames to consider: + // John Doe (Role - Department) - johndoe@constoso.com + // John - Doe - johndoe@constoso.com + // John Doe - john-doe@constoso.com + // John Doe - john-doe@constoso.org-name.com + + // A different way of implementing this is by sending user's email directly to STS in 'username' property but that would cause incompatibility + // with saved connection profiles and reading from settings.json, therefore this solution is used as of now. + + string emailSeparator = " - "; + string username = @params.UserName; + if (username.Contains(emailSeparator)) + { + int startIndex = username.LastIndexOf(emailSeparator) + emailSeparator.Length; + username = username.Substring(startIndex); + } + if (!Utils.isValidEmail(username)) { SqlToolsLogger.Pii($"{nameof(Authenticator)}.{nameof(GetTokenAsync)} | Unexpected username format, email not retreived: {@params.UserName}. " + @@ -69,33 +86,41 @@ namespace Microsoft.SqlTools.Authentication throw new Exception($"Invalid email address format for user: [{username}] received for Azure Active Directory authentication."); } - do + if (accounts.MoveNext()) { - IAccount? currentVal = accounts.Current; - if (string.Compare(username, currentVal.Username, StringComparison.InvariantCultureIgnoreCase) == 0) + do { - account = currentVal; - SqlToolsLogger.Verbose($"{nameof(Authenticator)}.{nameof(GetTokenAsync)} | User account found in MSAL Cache: {account.HomeAccountId}"); - break; + IAccount? currentVal = accounts.Current; + if (string.Compare(username, currentVal.Username, StringComparison.InvariantCultureIgnoreCase) == 0) + { + account = currentVal; + SqlToolsLogger.Verbose($"{nameof(Authenticator)}.{nameof(GetTokenAsync)} | User account found in MSAL Cache: {account.HomeAccountId}"); + break; + } } - } - while (accounts.MoveNext()); + while (accounts.MoveNext()); - if (null != account) - { - try + if (null != account) { - // Fetch token silently - var result = await publicClientApplication.AcquireTokenSilent(@params.Scopes, account) - .ExecuteAsync(cancellationToken: cancellationToken) - .ConfigureAwait(false); - accessToken = new AccessToken(result!.AccessToken, result!.ExpiresOn); + try + { + // Fetch token silently + var result = await publicClientApplication.AcquireTokenSilent(@params.Scopes, account) + .ExecuteAsync(cancellationToken: cancellationToken) + .ConfigureAwait(false); + accessToken = new AccessToken(result!.AccessToken, result!.ExpiresOn); + } + catch (Exception e) + { + SqlToolsLogger.Verbose($"{nameof(Authenticator)}.{nameof(GetTokenAsync)} | Silent authentication failed for resource {@params.Resource} for ConnectionId {@params.ConnectionId}."); + SqlToolsLogger.Error(e); + throw; + } } - catch (Exception e) + else { - SqlToolsLogger.Verbose($"{nameof(Authenticator)}.{nameof(GetTokenAsync)} | Silent authentication failed for resource {@params.Resource} for ConnectionId {@params.ConnectionId}."); - SqlToolsLogger.Error(e); - throw; + SqlToolsLogger.Error($"{nameof(Authenticator)}.{nameof(GetTokenAsync)} | Account not found in MSAL cache for user."); + throw new Exception($"User account '{username}' not found in MSAL cache, please add linked account or refresh account credentials."); } } else diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Authentication/AuthenticatorTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Authentication/AuthenticatorTests.cs new file mode 100644 index 00000000..5da4adf8 --- /dev/null +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Authentication/AuthenticatorTests.cs @@ -0,0 +1,50 @@ +// +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +// + +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.SqlTools.Authentication; +using NUnit.Framework; + +namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Authentication +{ + [TestFixture] + public class AuthenticatorTests + { + [Test] + [TestCase("John Doe - johndoe@constoso.com", "johndoe@constoso.com")] + [TestCase("John Doe - john-doe@constoso.com", "john-doe@constoso.com")] + [TestCase("John Doe (Manager - Sales) - johndoe@constoso.com", "johndoe@constoso.com")] + [TestCase("John - Doe (Manager - Sales) - john-doe@constoso.com", "john-doe@constoso.com")] + [TestCase("John Doe - johndoe@constoso-sales.com", "johndoe@constoso-sales.com")] + [TestCase("johndoe@constoso.com", "johndoe@constoso.com")] + [TestCase("johndoe@constoso-sales.com", "johndoe@constoso-sales.com")] + public async Task GetTokenAsyncExtractsEmailSuccessfully(string username, string expectedEmail) + { + Authenticator authenticator = new Authenticator(new SqlTools.Authentication.Utility.AuthenticatorConfiguration( + Guid.NewGuid().ToString(), "AppName", ".", "dummyCacheFile"), () => ("key", "iv")); + try + { + await authenticator.GetTokenAsync(new AuthenticationParams(AuthenticationMethod.ActiveDirectoryInteractive, + "https://login.microsoftonline.com/", + "common", + "https://database.windows.net/", + new string[] { + "https://database.windows.net/.default" + }, + username, + Guid.Empty), + CancellationToken.None); + Assert.Fail("Expected exception did not occur."); + } + catch (Exception e) + { + Assert.False(e.Message.StartsWith("Invalid email address format", StringComparison.OrdinalIgnoreCase), $"Email address format should be correct, message received: {e.Message}"); + Assert.True(e.Message.Contains($"User account '{expectedEmail}' not found in MSAL cache, please add linked account or refresh account credentials."), $"Expected error did not occur, message received: {e.Message}"); + } + } + } +}