New-DbaDbUser doesn't respect -Database param for system dbs

[lowlydba]

Verified issue does not already exist?

Yes

What error did you receive?

When creating a user in a system database, the function claims that specifying a Database will override the default mode of excluding system databases, but it does not.

The param description:

dbatools/public/New-DbaDbUser.ps1

Lines 25 to 26 in ffb7b37

	.PARAMETER IncludeSystem
	If this switch is enabled, the user will be added to system databases. This switch will be ignored if -Database is used.

The logic:

dbatools/public/New-DbaDbUser.ps1

Lines 155 to 157 in ffb7b37

	if ($Database) { $getDbParam.Database = $Database }
	if ($ExcludeDatabase) { $getDbParam.ExcludeDatabase = $ExcludeDatabase }
	if (-not ($IncludeSystem)) { $getDbParam.ExcludeSystem = $True }

Steps to Reproduce

# No change
New-DbaDbUser -SqlInstance localhost -Database master -User lowlydba2 -Login lowlydba -DefaultSChema "Information_SCHEMA" -Verbose 
# Works
New-DbaDbUser -SqlInstance localhost -Database master -User lowlydba2 -Login lowlydba -DefaultSChema "Information_SCHEMA" -Verbose -IncludeSystem

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

v2.0.0 (congrats!)

Other details or mentions

No response

What PowerShell host was used when producing this error

PowerShell Core (pwsh.exe)

PowerShell Host Version

N/A

SQL Server Edition and Build number

N/A

.NET Framework Version

N/A

[wsmelton]

Why would you need to create a user in a system database?

I would only see that the docs need updating here as creating users in the system databases is not a normal pattern and unless there is some unwavering logic behind needing to change to allow it without the include flag, it should stay as-is.

[lowlydba]

Why would you need to create a user in a system database?

I would only see that the docs need updating here as creating users in the system databases is not a normal pattern and unless there is some unwavering logic behind needing to change to allow it without the include flag, it should stay as-is.

In my case, it is in a CI pipeline and it doesn't make sense to create a user database just to test adding a user in an ephemeral instance.

I purposely didn't propose a fix since I don't have any preference, so a docs update works for me.

[wsmelton]

Did find we create a database for the command's test.

https://github.com/dataplat/dbatools/blob/development/tests/New-DbaDbUser.Tests.ps1

I think docs and test updates are warranted. Adding an example in docs and then testing that in the command's test will be good.

[andreasjordan]

I think the easierst is to align the code with the docs by changing the order of some lines. So I opened a PR for that. If onyone prefers a different solution - feel free to provide one.