Connect-DbaInstance / Invoke-DbaQuery - Open a correct new connection when ConnectAsUserName is used and fix usage of AppendConnectionString (#9680)

Author: andreasjordan

public/Connect-DbaInstance.ps1

@@ -289,6 +289,7 @@ function Connect-DbaInstance {
 
     #>
     [CmdletBinding()]
+    [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingConvertToSecureStringWithPlainText", "")]
     param (
         [Parameter(Mandatory, ValueFromPipeline)]
         [Alias("Connstring", "ConnectionString")]
@@ -592,6 +593,7 @@ function Connect-DbaInstance {
                 # Currently only if we have a different Database or have to switch to a NonPooledConnection or using a specific StatementTimeout or using ApplicationIntent
                 # We do not test for SqlCredential as this would change the behavior compared to the legacy code path
                 $copyContext = $false
+                $createNewConnection = $false
                 if ($Database) {
                     Write-Message -Level Debug -Message "Database [$Database] provided."
                     if (-not $inputObject.ConnectionContext.CurrentDatabase) {
@@ -602,6 +604,10 @@ function Connect-DbaInstance {
                     if ($inputObject.ConnectionContext.CurrentDatabase -ne $Database) {
                         Write-Message -Level Verbose -Message "Database [$Database] provided. Does not match ConnectionContext.CurrentDatabase [$($inputObject.ConnectionContext.CurrentDatabase)], copying ConnectionContext and setting the CurrentDatabase"
                         $copyContext = $true
+                        if ($inputObject.ConnectionContext.ConnectAsUserName -ne '') {
+                            Write-Message -Level Debug -Message "Using ConnectAsUserName [$($inputObject.ConnectionContext.ConnectAsUserName)], so changing database context is not possible without loosing this information. We will create a new connection targeting database [$Database]"
+                            $createNewConnection = $true
+                        }
                     }
                 }
                 if ($ApplicationIntent -and $inputObject.ConnectionContext.ApplicationIntent -ne $ApplicationIntent) {
@@ -620,7 +626,15 @@ function Connect-DbaInstance {
                     Write-Message -Level Verbose -Message "DedicatedAdminConnection provided. Does not match ConnectionContext.ServerInstance, copying ConnectionContext and setting the ServerInstance"
                     $copyContext = $true
                 }
-                if ($copyContext) {
+                if ($createNewConnection) {
+                    $isNewConnection = $true
+                    $secStringPassword = ConvertTo-SecureString -String $inputObject.ConnectionContext.ConnectAsUserPassword -AsPlainText -Force
+                    $serverCredentialFromSMO = New-Object System.Management.Automation.PSCredential($inputObject.ConnectionContext.ConnectAsUserName, $secStringPassword)
+                    $connectParams = $PSBoundParameters
+                    $connectParams.SqlInstance = $inputObject.Name
+                    $connectParams.SqlCredential = $serverCredentialFromSMO
+                    $server = Connect-DbaInstance @connectParams
+                } elseif ($copyContext) {
                     $isNewConnection = $true
                     $connContext = $inputObject.ConnectionContext.Copy()
                     if ($ApplicationIntent) {

public/Invoke-DbaQuery.ps1

@@ -169,7 +169,6 @@ function Invoke-DbaQuery {
         See https://learn.microsoft.com/en-us/dotnet/framework/data/adonet/sql/sqlclient-support-for-high-availability-disaster-recovery#connecting-with-multisubnetfailover
     #>
     [CmdletBinding(DefaultParameterSetName = "Query")]
-    [Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingConvertToSecureStringWithPlainText", "")]
     param (
         [Parameter(ValueFromPipeline)]
         [Parameter(ParameterSetName = 'Query', Position = 0)]
@@ -406,83 +405,65 @@ function Invoke-DbaQuery {
             # We suppress the verbosity of all other functions in order to be sure the output is consistent with what you get, e.g., executing the same in SSMS
             Write-Message -Level Debug -Message "SqlInstance passed in, will work on: $instance"
             try {
+                # We got another nightmare to solve, but fortunately @andreasjordan is "the king"
+                # So. Here we are with a passed down "Server" instance. Problem is, we got two VERY different
+                # libraries built with VERY different agendas. And we want to get the best of both worlds.
+                # This is SUCH a nightmare because Invoke-DbaQuery is THE ONLY function that CAN use a Connection
+                # (Microsoft.Data.SqlClient.SqlConnection) which can be used to run PARAMETRIZED queries.
+                # So, recap of the recap:
+                #  1. Microsoft.Data.SqlClient.SqlConnection:
+                #     PRO: is the only connection that can use Microsoft.Data.SqlClient.SqlCommand
+                #          that can use [Microsoft.Data.SqlClient.SqlParameter]s
+                #     CON: when using integrated auth, as in Integrated Security=True, cannot specify a DIFFERENT user than the current logged in one
+                #  2. Microsoft.SqlServer.Management.Smo.Server
+                #     PRO: can specify a DIFFERENT user than the current logged in one via ConnectAsUser, ConnectAsUserName, ConnectAsUserPassword when Integrated Security=True
+                #     CON: cannot use Microsoft.Data.SqlClient.SqlCommand nor [Microsoft.Data.SqlClient.SqlParameter]s
+                #
+                # Till here, everything is clear: we want to reuse connection if we're sure the target is "95%" adherent to the supposed one
+                # But, and that's a big but, the magic in Invoke-DbaQuery is making a Microsoft.SqlServer.Management.Smo.Server connection happen, let it "bleed" through here
+                # land in Invoke-DbaAsync untouched, where it gets magically converted to a Microsoft.Data.SqlClient.SqlConnection, and we get the best of both worlds.
+                # Thing is, the "magic" is rather ... not so magic. What it happens is that when the connection from Microsoft.SqlServer.Management.Smo.Server is "Open",
+                # everything works fine, while if it's "Closed", Microsoft.Data.SqlClient.SqlConnection rehydrates a connection using the ConnectionString of Microsoft.SqlServer.Management.Smo.Server.
+                # Microsoft.SqlServer.Management.Smo.Server is cheating though, because there's no connectionstring parameter that holds "log on as a different user", so, what happens is that
+                # when Microsoft.Data.SqlClient.SqlConnection picks it up, rehydrates the connection that holds Integrated Security=True, and the information on "log on as a different user" is lost in
+                # translation.
+                # As anticipated, this happens ONLY when the connection is "Closed", because when it's "Open", no rehydration is needed, and everything works out of the box.
+                # Now, another fancy thing: we want to use connection pooling by default, because pooling connection is more performant.
+                # But, and that's the big but, when connection is pooled, it gets put in the "Closed" state which translates to "back to the pool, ready to be reused", as soon as no commands are actively used.
+                # In dbatools world, that means pretty much that when we are here, it's always "Closed" UNLESS -NonPooledConnection is $true on Connect-DbaInstance, and that's why when we don't land here
+                # we create a new instance with NonPooledConnection = $true ( see #8491 for details, also #7725 is still relevant)
+                # If -NonPooled is passed, we instruct Microsoft.SqlServer.Management.Smo.Server we DON'T want pooling, so the connection stays "Open" (there's no pool to put it back),
+                # and when it's "Open" we can leverage the fact that we already established a connection, verified the certificate, did the login handshake, etc AND when casting
+                # to Microsoft.Data.SqlClient.SqlConnection it enables us to leverage [Microsoft.Data.SqlClient.SqlParameter]s !
+                #
+                # Again, here we are, but we cannot use the connection when an information is lost, which is that we are:
+                #   - Integrated Security=True
+                #   - using ConnectAsUser, ConnectAsUserName, ConnectAsUserPassword to "log on as a different user"
+
                 $startedWithAnOpenConnection = # we want to bypass Connect-DbaInstance if
                 ($instance.InputObject.GetType().Name -eq "Server") -and # we have Server SMO object and
                 (-not $ReadOnly) -and # no readonly intent is requested and
-                (-not $Database -or $instance.InputObject.ConnectionContext.DatabaseName -eq $Database)  # the database is not set or the currently connected
+                (-not $Database -or $instance.InputObject.ConnectionContext.DatabaseName -eq $Database) -and # the database is not set or the currently connected and
+                (-not $AppendConnectionString) -and # we don't use AppendConnectionString and
+                ($instance.InputObject.ConnectionContext.ConnectAsUserName -eq '') # we don't use a DIFFERENT operating system user than the current logged in
                 if ($startedWithAnOpenConnection) {
-                    Write-Message -Level Debug -Message "Current connection can be reused"
-                    # We got another nightmare to solve, but fortunately @andreasjordan is "the king"
-                    # So. Here we are with a passed down "Server" instance. Problem is, we got two VERY different
-                    # libraries built with VERY different agendas. And we want to get the best of both worlds.
-                    # This is SUCH a nightmare because Invoke-DbaQuery is THE ONLY function that CAN use a Connection
-                    # (Microsoft.Data.SqlClient.SqlConnection) which can be used to run PARAMETRIZED queries.
-                    # So, recap of the recap:
-                    #  1. Microsoft.Data.SqlClient.SqlConnection:
-                    #     PRO: is the only connection that can use Microsoft.Data.SqlClient.SqlCommand
-                    #          that can use [Microsoft.Data.SqlClient.SqlParameter]s
-                    #     CON: when using integrated auth, as in Integrated Security=True, cannot specify a DIFFERENT user than the current logged in one
-                    #  2. Microsoft.SqlServer.Management.Smo.Server
-                    #     PRO: can specify a DIFFERENT user than the current logged in one via ConnectAsUser, ConnectAsUserName, ConnectAsUserPassword when Integrated Security=True
-                    #     CON: cannot use Microsoft.Data.SqlClient.SqlCommand nor [Microsoft.Data.SqlClient.SqlParameter]s
-                    #
-                    # Till here, everything is clear: we want to reuse connection if we're sure the target is "95%" adherent to the supposed one
-                    # But, and that's a big but, the magic in Invoke-DbaQuery is making a Microsoft.SqlServer.Management.Smo.Server connection happen, let it "bleed" through here
-                    # land in Invoke-DbaAsync untouched, where it gets magically converted to a Microsoft.Data.SqlClient.SqlConnection, and we get the best of both worlds.
-                    # Thing is, the "magic" is rather ... not so magic. What it happens is that when the connection from Microsoft.SqlServer.Management.Smo.Server is "Open",
-                    # everything works fine, while if it's "Closed", Microsoft.Data.SqlClient.SqlConnection rehydrates a connection using the ConnectionString of Microsoft.SqlServer.Management.Smo.Server.
-                    # Microsoft.SqlServer.Management.Smo.Server is cheating though, because there's no connectionstring parameter that holds "log on as a different user", so, what happens is that
-                    # when Microsoft.Data.SqlClient.SqlConnection picks it up, rehydrates the connection that holds Integrated Security=True, and the information on "log on as a different user" is lost in
-                    # translation.
-                    # As anticipated, this happens ONLY when the connection is "Closed", because when it's "Open", no rehydration is needed, and everything works out of the box.
-                    # Now, another fancy thing: we want to use connection pooling by default, because pooling connection is more performant.
-                    # But, and that's the big but, when connection is pooled, it gets put in the "Closed" state which translates to "back to the pool, ready to be reused", as soon as no commands are actively used.
-                    # In dbatools world, that means pretty much that when we are here, it's always "Closed" UNLESS -NonPooledConnection is $true on Connect-DbaInstance, and that's why when we don't land here
-                    # we create a new instance with NonPooledConnection = $true ( see #8491 for details, also #7725 is still relevant)
-                    # If -NonPooled is passed, we instruct Microsoft.SqlServer.Management.Smo.Server we DON'T want pooling, so the connection stays "Open" (there's no pool to put it back),
-                    # and when it's "Open" we can leverage the fact that we already established a connection, verified the certificate, did the login handshake, etc AND when casting
-                    # to Microsoft.Data.SqlClient.SqlConnection it enables us to leverage [Microsoft.Data.SqlClient.SqlParameter]s !
-                    #
-                    # Again, here we are, but we cannot use the connection when an information is lost, which is that we are:
-                    #   - Integrated Security=True
-                    #   - using ConnectAsUser, ConnectAsUserName, ConnectAsUserPassword to "log on as a different user"
-
-                    if ($instance.InputObject.ConnectionContext.ConnectAsUserName -ne '') {
-                        Write-Message -Level Debug -Message "Current connection cannot be reused because logging in as a different user"
-                        # We rebuild correct credentials from SMO informations
-                        $secStringPassword = ConvertTo-SecureString $instance.InputObject.ConnectionContext.ConnectAsUserPassword -AsPlainText -Force
-                        [PSCredential]$serverCredentialFromSMO = New-Object System.Management.Automation.PSCredential($instance.InputObject.ConnectionContext.ConnectAsUserName, $secStringPassword)
-                        $connDbaInstanceParams = @{
-                            SqlInstance            = $instance
-                            SqlCredential          = $serverCredentialFromSMO
-                            Database               = $Database
-                            NonPooledConnection    = $true           # see #8491 for details, also #7725 is still relevant
-                            Verbose                = $false
-                            AppendConnectionString = $AppendConnectionString
-                        }
-                        if ($ReadOnly) {
-                            $connDbaInstanceParams.ApplicationIntent = "ReadOnly"
-                        }
-
-                        $server = Connect-DbaInstance @connDbaInstanceParams
-
-                    } else {
-                        Write-Message -Level Debug -Message "Current connection will be reused"
-                        $server = $instance.InputObject
-                    }
-
+                    Write-Message -Level Debug -Message "Current connection will be reused"
+                    $server = $instance.InputObject
                 } else {
                     $connDbaInstanceParams = @{
-                        SqlInstance            = $instance
-                        SqlCredential          = $SqlCredential
-                        Database               = $Database
-                        NonPooledConnection    = $true           # see #8491 for details, also #7725 is still relevant
-                        Verbose                = $false
-                        AppendConnectionString = $AppendConnectionString
+                        SqlInstance         = $instance
+                        SqlCredential       = $SqlCredential
+                        Database            = $Database
+                        NonPooledConnection = $true           # see #8491 for details, also #7725 is still relevant
+                        Verbose             = $false
                     }
                     if ($ReadOnly) {
                         $connDbaInstanceParams.ApplicationIntent = "ReadOnly"
                     }
+                    if ($AppendConnectionString) {
+                        $connDbaInstanceParams.AppendConnectionString = $AppendConnectionString
+                        $connDbaInstanceParams.SqlInstance = "$instance" # pass down "as a string" so it's forced to open a new one
+                    }
                     $server = Connect-DbaInstance @connDbaInstanceParams
                 }
             } catch {