Fix email extraction from username (#2075)

microsoft · May 30, 2023 · b4f7bd0 · b4f7bd0
1 parent 9798866
commit b4f7bd0
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 24 deletions.
diff --git a/src/Microsoft.SqlTools.Authentication/Authenticator.cs b/src/Microsoft.SqlTools.Authentication/Authenticator.cs
@@ -13,7 +13,7 @@ namespace Microsoft.SqlTools.Authentication
     /// <summary>
     /// Provides APIs to acquire access token using MSAL.NET v4 with provided <see cref="AuthenticationParams"/>.
     /// </summary>
-    public class Authenticator: IAuthenticator
+    public class Authenticator : IAuthenticator
     {
         private AuthenticatorConfiguration configuration;
 
@@ -58,44 +58,69 @@ public Authenticator(AuthenticatorConfiguration configuration, MsalEncryptedCach
                 IEnumerator<IAccount>? 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}. " +
                             $"Accepted formats are: 'johndoe@org.com' or 'John Doe - johndoe@org.com'.");
                         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
@@ -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}");
+            }
+        }
+    }
+}