`New-DbaDbUser` - Multiple `-SqlInstance` Without `-Database` Doesn't Add User To All Dbs

[mattcargile]

Verified issue does not already exist?

I have searched and found no existing issue

What error did you receive?

No error message. Login/User isn't added on all non-system databases.

Steps to Reproduce

New-DbaDbUser -SqlInstance db1, db2 -User 'User'

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

Pulled from recent development.

Other details or mentions

Some how $getdbparam retains multiple databases in the Database key after the first full iteration of the $instance loop. I'm not entirely sure how this happens. Maybe something with the handling of the cloning.

I was playing with the below breakpoint.

Set-PSBreakpoint -Script public\New-DbaDbUser.ps1 -Line 166 -Action { if($getdbparam.Database.Count -ne 1) { break } }

What PowerShell host was used when producing this error

PowerShell Core (pwsh.exe)

PowerShell Host Version

Name                           Value
----                           -----
PSVersion                      7.4.1
PSEdition                      Core
GitCommitId                    7.4.1
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 2017 (RTM-CU28) (KB5008084) - 14.0.3430.2 (X64) 
	Dec 17 2021 14:30:27 
	Copyright (C) 2017 Microsoft Corporation
	Enterprise Edition: Core-based Licensing (64-bit) on Windows Server 2016 Standard 10.0 <X64> (Build 14393: ) (Hypervisor)

.NET Framework Version

.NET 8.0.1

[andreasjordan]

Hi Matt,

here is (most likely) the bug that was introduced in #9078:

            #This block is required so that correct error message can be returned to the user when incorrect database name is given or the database doesn't exists in the server.
            if (-not $Database) {
                $Database = $databases.Name
            }

This only works in the first run, where $Database is not set and thus all the databases on that instance are used.
On the second run, $Database is set (by the first run) and thus only the databases on the first instance are used.

To get this to work, $Database must remain unchanged, So for the loop, we need another variable which holds that databases that should be processed for this instance.

Maybe @sqlarticles can provide a fix for that. Or I can have a look in the next week or two.

[sqlarticles]

Thanks @andreasjordan for looking into this.

I will try and reproduce the error and release a fix.

[wsmelton]

The test in that PR isn't actually validating the expected result. It only looks at the login name existing, it should also include validating it exist in all the databases too.

[andreasjordan]

I had a look at the code and some other changes to the command in the last months. I don't know how to say that in a polite way so please excuse me, but this command is all messed up. There are a lot of bugs and the code in general is not very nice.

I will try refactor it again to clean things a little bit.