Don't default to AzureMFA (#2549)

* When parsing a connection string don't default to AzureMFA if the auth method is not, SqlLogin or Integrated or AzureMFA then return null as the AuthenticationType

* Updating test, PR feedback

---------

Co-authored-by: Allan Cascante <[email protected]>
Co-authored-by: Benjin Dubishar (from Dev Box) <[email protected]>

Author: 3 people

src/Microsoft.SqlTools.ServiceLayer/Connection/ConnectionService.cs

@@ -1750,15 +1750,30 @@ public ConnectionDetails ParseConnectionString(string connectionString)
         {
             SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder(connectionString);
 
+            string mappedAuthenticationType;
+            if (builder.IntegratedSecurity)
+            {
+                mappedAuthenticationType = "Integrated";
+            } else if (builder.Authentication == SqlAuthenticationMethod.ActiveDirectoryInteractive)
+            {
+                mappedAuthenticationType = "AzureMFA";
+            }
+            else if (builder.Authentication == SqlAuthenticationMethod.SqlPassword || builder.Authentication == SqlAuthenticationMethod.NotSpecified)
+            {
+                mappedAuthenticationType = "SqlLogin";
+            }
+            else
+            {
+                mappedAuthenticationType = builder.Authentication.ToString();
+            }
+
             // Set defaults as per MSSQL connection property defaults, not SqlClient's Connection string buider defaults
             ConnectionDetails details = new ConnectionDetails()
             {
                 ApplicationIntent = defaultBuilder.ApplicationIntent != builder.ApplicationIntent ? builder.ApplicationIntent.ToString() : null,
                 ApplicationName = defaultBuilder.ApplicationName != builder.ApplicationName ? builder.ApplicationName : ApplicationName,
                 AttachDbFilename = defaultBuilder.AttachDBFilename != builder.AttachDBFilename ? builder.AttachDBFilename.ToString() : null,
-                AuthenticationType = builder.IntegratedSecurity ? "Integrated" :
-                    ((builder.Authentication == SqlAuthenticationMethod.SqlPassword || builder.Authentication == SqlAuthenticationMethod.NotSpecified)
-                    ? "SqlLogin" : "AzureMFA"),
+                AuthenticationType = mappedAuthenticationType,
                 ConnectRetryCount = defaultBuilder.ConnectRetryCount != builder.ConnectRetryCount ? builder.ConnectRetryCount : 1,
                 ConnectRetryInterval = defaultBuilder.ConnectRetryInterval != builder.ConnectRetryInterval ? builder.ConnectRetryInterval : 10,
                 ConnectTimeout = defaultBuilder.ConnectTimeout != builder.ConnectTimeout ? builder.ConnectTimeout : 30,

test/Microsoft.SqlTools.ServiceLayer.UnitTests/Connection/ConnectionServiceTests.cs

@@ -1863,18 +1863,24 @@ public void ParseConnectionStringTest_AuthTypes()
             details = service.ParseConnectionString(connectionString);
             Assert.That(details.AuthenticationType, Is.EqualTo("Integrated"));
 
-            // AAD auth types boil down to AzureMFA
-            connectionString = "Server=tcp:{servername},1433;Initial Catalog={databasename};Authentication=ActiveDirectoryIntegrated;";
+            // AAD auth types
+            // ActiveDirectoryInteractive is mapped to to AzureMFA
+            connectionString = "Server=tcp:{servername},1433;Initial Catalog={databasename};Authentication=ActiveDirectoryInteractive;";
             details = service.ParseConnectionString(connectionString);
             Assert.That(details.AuthenticationType, Is.EqualTo("AzureMFA"));
 
-            connectionString = "Server=tcp:{servername},1433;Initial Catalog={databasename};Authentication=ActiveDirectoryInteractive;";
+            // Other AAD types are unchanged
+            connectionString = "Server=tcp:{servername},1433;Initial Catalog={databasename};Authentication=ActiveDirectoryIntegrated;";
             details = service.ParseConnectionString(connectionString);
-            Assert.That(details.AuthenticationType, Is.EqualTo("AzureMFA"));
+            Assert.That(details.AuthenticationType, Is.EqualTo("ActiveDirectoryIntegrated"));
 
             connectionString = "Server=tcp:{servername},1433;Initial Catalog={databasename};Authentication=ActiveDirectoryDefault;";
             details = service.ParseConnectionString(connectionString);
-            Assert.That(details.AuthenticationType, Is.EqualTo("AzureMFA"));
+            Assert.That(details.AuthenticationType, Is.EqualTo("ActiveDirectoryDefault"));
+
+            // Invalid throws
+            connectionString = "Server=tcp:{servername},1433;Initial Catalog={databasename};Authentication=InvalidAuthType;";
+            Assert.Throws<ArgumentException>(() => service.ParseConnectionString(connectionString), "Invalid value for key 'authentication'.");
         }
 
         [Test]