New-DbaConnectionString Multiple Errors/Issues

[MrTCS]

Verified issue does not already exist?

I have searched and found no existing issue

What error did you receive?

Main Error

Even though it's throwing an error it's still creating the connection string.

Any attempt to set values for TrustServerCertificate and EncryptConnection then force the use of a credential.

Adding a value for SQLCredential causes another issue requiring ApplicationIntent to be required

Once all that is supplied we are back to the first error, but now the connection string exposes the username and password of the credential, since Integrated Security is no longer a valid option

Steps to Reproduce

New-DbaConnectionString -SqlInstance

Please confirm that you are running the most recent version of dbatools

2.1.18

Other details or mentions

No response

What PowerShell host was used when producing this error

PowerShell Core (pwsh.exe), Windows PowerShell (powershell.exe), VS Code (terminal)

PowerShell Host Version

Name Value

---

PSVersion 7.4.3
PSEdition Core
GitCommitId 7.4.3
OS Microsoft Windows 10.0.19045
Platform Win32NT
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0

SQL Server Edition and Build number

Microsoft SQL Server 2022 (RTM-CU13) (KB5036432) - 16.0.4125.3 (X64) May 1 2024 15:05:56 Copyright (C) 2022 Microsoft Corporation Enterprise Edition: Core-based Licensing (64-bit) on Windows Server 2019 Standard 10.0 (Build 17763: ) (Hypervisor)

.NET Framework Version

.NET 8.0.6

[niphlod]

seems a mismatch here

[ValidateSet('Mandatory', 'Optional', 'Strict', 'True', 'False')]
[switch]$EncryptConnection = (Get-DbatoolsConfigValue -FullName 'sql.connection.encrypt'),

first, can a switch be validated as a set ? don't think so, but don't take me from granted.

If it "remains" a switch, when newer versions are targeted , our "encrypt yes/no" should be declined into the "Mandatory/Optional" new costraint. Supporting "Strict" should go in yet another parameter.
If it becomes a set, we can accomodate for "Strict", too, and if it's not passed, we can take the sql.connection.encrypt preference and decline it into "Mandatory/Optional" when targeting the newer library.

@andreasjordan : do you concur ?

[MrTCS]

I get similar behavior when using Connect-DbaInstance, one of the main differences in the final result though is it at least outputs the connection string with Integrated Security instead of displaying the User Name and Password

[andreasjordan]

Give me around 24 hours and I will have a look at it.

[MrTCS]

I got to spend a little bit more time looking into this and found that setting the following to false results in no error being thrown.

Set-DbatoolsConfig -FullName sql.connection.encrypt -Value $false

I still couldn't figure out why setting some parameters, like -TrustServerCert $true, ended up requiring -SqlCredential to be required instead of being able to use integrated auth. But setting the following also got around that.

Set-DbatoolsConfig -FullName sql.connection.trustcert -Value $true

[andreasjordan]

I changed the parameter in this pull request: #9143

We had a discussion about the best way, and maybe my proposed way was not the best option.

See also my comments here: #9113

The problem is that:

• SqlConnectionInfo.EncryptConnection is a bool (and we use this in Connect-DbaInstance)
• SqlConnectionStringBuilder.Encrypt is of type SqlConnectionEncryptOption (and we use this in New-DbaConnectionString)

I still think we should use a bool in Connect-DbaInstance to have a simple yes/no for encryption that is the same for all supported versions of sql server (as we only know the version after we connected to it). And I think we don't have a problem with the current implementation in Connect-DbaInstance.

So we should change New-DbaConnectionString. The current parameter EncryptConnection should be the same as in Connect-DbaInstance, so we have to remove the validate set.
To be able to use the new options with the new versions of sql server, we need to add a new parameter with the validate set. We should name the parameter "EncryptOption" as this is a good part of the typename "SqlConnectionEncryptOption".

If you think this is the right direction, I can code that and open a pull request.

[niphlod]

in my mind, at veeeery high level, the point stems from the fact that a switch cannot accomodate the newer library options.
So, for a low-level thing like new-dbaconnectionstring, MAYBE we should drop the "old way of thinking boolean" and let the full breadth of allowed params. If it's not passed explicitely, it should be dbatools that, if it chooses so, needs to get the preference value (that is shared among other functions as well) and "do the dance of mapping", using the correct value allowed for the underlying parameter of the sql library.

As for adapting to the "newer standard" in other dbatools places, assumed the new sql server library allows for more values now, it seems that our "bool" preference is falling short (on top of, if I understood the referenced comments, being able to use both "new" and "old" library, or different libraries alltogether).

[andreasjordan]

Keep in mind that the parameter has a configuration item connected to it.

This was a string with "Mandatory" as the default. I think that caused problems so I changed that to bool here:
f3fad5a

If that was not the best way, we can revert that. But what value to use as a default for new installations?

[niphlod]

IMHO we need to reconvene on that point (with the usual suspects, maybe in slack, too).
It seems that MS is shifting towards the enum (and keeping it) rather than the switch, so in the long run we should adhere to the enum instead of a bool.

[andreasjordan]

This is basically the code we run inside of Connect-DbaInstance:

$serverName = 'sql01\sql2022'
$EncryptConnection = Get-DbatoolsConfigValue -FullName 'sql.connection.encrypt'
$TrustServerCertificate = Get-DbatoolsConfigValue -FullName 'sql.connection.trustcert'
$sqlConnectionInfo = New-Object -TypeName Microsoft.SqlServer.Management.Common.SqlConnectionInfo -ArgumentList $serverName
$sqlConnectionInfo.EncryptConnection = $EncryptConnection
$sqlConnectionInfo.TrustServerCertificate = $TrustServerCertificate
$serverConnection = New-Object -TypeName Microsoft.SqlServer.Management.Common.ServerConnection -ArgumentList $sqlConnectionInfo
$server = New-Object -TypeName Microsoft.SqlServer.Management.Smo.Server -ArgumentList $serverConnection
$null = $server.ConnectionContext.ExecuteWithResults("SELECT 'dbatools is opening a new connection'")

We use Microsoft.SqlServer.Management.Common.SqlConnectionInfo and Microsoft.SqlServer.Management.Common.ServerConnection which both use EncryptConnection as bool. But they both have a parameter StrictEncryption, which is also a bool.

What if we add this parameter also to Connect-DbaInstance and New-DbaConnectionString?

In Connect-DbaInstance we just pass the two bools to the properties. In New-DbaConnectionString we build a ruleset to convert the two bools into a string ('Mandatory', 'Optional', 'Strict', 'True', 'False'). I think if StrictEncryption is $false, then we just convert the bool value from EncryptConnection into the string 'True' or 'False'. If StrictEncryption is $true, then it should be 'Strict' I think, but not sure.

In case we need a real quick fix with minimal change, I can just change $connStringBuilder['Encrypt'] = $EncryptConnection into $connStringBuilder['Encrypt'] = "$EncryptConnection".