Export-DbaUser - Refactor T-SQL User-Role Scripting to Eliminate Depe…

…ndencies (#9232)

public/Export-DbaUser.ps1

@@ -250,8 +250,6 @@ function Export-DbaUser {
                 $FilePath = Get-ExportFilePath -Path $PSBoundParameters.Path -FilePath $PSBoundParameters.FilePath -Type sql -ServerName $db.Parent.Name -Unique
             }
 
-            # Store roles between users so if we hit the same one we don't create it again
-            $roles = @()
             $stepCounter = 0
             foreach ($dbuser in $users) {
 
@@ -274,14 +272,69 @@ function Export-DbaUser {
                 }
 
                 try {
+                    <#
+                    In this approach, we do not maintain a variable to track the roles that have been scripted. Our method involves a
+                    consistent verification process for each user against the complete list of roles. This ensures that we dynamically
+                    include only the roles to which a user belongs. For example, consider two users: user1 is associated with role1 and
+                    role2, while user2 is associated with role1 and role3.
+
+                    Attempting to memorize the scripted roles could result in Transact-SQL (T-SQL) statements such as:
+
+                    IF NOT EXISTS (role1)
+                      CREATE ROLE role1
+                    IF NOT EXISTS (role2)
+                      CREATE ROLE role2
+                    IF NOT EXISTS (user1)
+                      CREATE USER user1
+                    ADD user1 TO role1
+                    ADD user1 TO role2
+
+                    -- And for another user:
+
+                    IF NOT EXISTS (role3)
+                      CREATE ROLE role3
+                    IF NOT EXISTS (user2)
+                      CREATE USER user2
+                    ADD user2 TO role1
+                    ADD user2 TO role3
+
+                    However, this script inadvertently introduces a dependency issue. To ensure user2 is properly configured, the script
+                    segment for user1 must be executed first due to the shared role1. To circumvent this issue and remove interdependencies,
+                    we opt to match each user against all potential roles. Consequently, roles are scripted per user membership, resulting
+                    in T-SQL like:
+
+                    IF NOT EXISTS (role1)
+                      CREATE ROLE role1
+                    IF NOT EXISTS (role2)
+                      CREATE ROLE role2
+                    IF NOT EXISTS (user1)
+                      CREATE USER user1
+                    ADD user1 TO role1
+                    ADD user1 TO role2
+
+                    -- And for another user:
+
+                    IF NOT EXISTS (role1)
+                      CREATE ROLE role1
+                    IF NOT EXISTS (role3)
+                      CREATE ROLE role3
+                    IF NOT EXISTS (user2)
+                      CREATE USER user2
+                    ADD user2 TO role1
+                    ADD user2 TO role3
+
+                    While this method may produce some redundant code (e.g., checking and creating role1 twice), it guarantees that each
+                    portion of the script is self-sufficient and can be executed independently of others. Therefore, users can selectively
+                    execute any segment of the script without concern for execution order or dependencies.
+                    #>
                     #Fixed Roles #Dependency Issue. Create Role, before add to role.
-                    foreach ($rolePermission in ($db.Roles | Where-Object { $_.IsFixedRole -eq $false })) {
-                        foreach ($rolePermissionScript in $rolePermission.Script($ScriptingOptionsObject)) {
-                            if ($rolePermission.ToString() -notin $roles) {
-                                $roles += , $rolePermission.ToString()
+                    foreach ($role in ($db.Roles | Where-Object { $_.IsFixedRole -eq $false })) {
+                        # Check if the user is a member of the role
+                        $isUserMember = $role.EnumMembers() | Where-Object { $_ -eq $dbuser.Name }
+                        if ($isUserMember) {
+                            foreach ($rolePermissionScript in $role.Script($ScriptingOptionsObject)) {
                                 $outsql += "$($rolePermissionScript.ToString())"
                             }
-
                         }
                     }
 

tests/Export-DbaUser.Tests.ps1

@@ -27,21 +27,42 @@ Describe "$commandname Integration Tests" -Tags "IntegrationTests" {
             $user2 = "dbatoolsci_exportdbauser_user2"
             $table = "dbatoolsci_exportdbauser_table"
             $role = "dbatoolsci_exportdbauser_role"
+
+            # For Dependencies elimination test
+            $login01 = "dbatoolsci_exportdbauser_login01"
+            $login02 = "dbatoolsci_exportdbauser_login02"
+            $user01 = "dbatoolsci_exportdbauser_user01"
+            $user02 = "dbatoolsci_exportdbauser_user02"
+            $role01 = "dbatoolsci_exportdbauser_role01"
+            $role02 = "dbatoolsci_exportdbauser_role02"
+            $role03 = "dbatoolsci_exportdbauser_role03"
+
             $server = Connect-DbaInstance -SqlInstance $script:instance1
             $null = $server.Query("CREATE DATABASE [$dbname]")
 
             $securePassword = $(ConvertTo-SecureString -String "GoodPass1234!" -AsPlainText -Force)
             $null = New-DbaLogin -SqlInstance $script:instance1 -Login $login -Password $securePassword
             $null = New-DbaLogin -SqlInstance $script:instance1 -Login $login2 -Password $securePassword
+            $null = New-DbaLogin -SqlInstance $script:instance1 -Login $login01 -Password $securePassword
+            $null = New-DbaLogin -SqlInstance $script:instance1 -Login $login02 -Password $securePassword
 
             $db = Get-DbaDatabase -SqlInstance $script:instance1 -Database $dbname
             $null = $db.Query("CREATE USER [$user] FOR LOGIN [$login]")
             $null = $db.Query("CREATE USER [$user2] FOR LOGIN [$login2]")
+            $null = $db.Query("CREATE USER [$user01] FOR LOGIN [$login01]")
+            $null = $db.Query("CREATE USER [$user02] FOR LOGIN [$login02]")
             $null = $db.Query("CREATE ROLE [$role]")
+            $null = $db.Query("CREATE ROLE [$role01]")
+            $null = $db.Query("CREATE ROLE [$role02]")
+            $null = $db.Query("CREATE ROLE [$role03]")
 
             $null = $db.Query("CREATE TABLE $table (C1 INT);")
             $null = $db.Query("GRANT SELECT ON OBJECT::$table TO [$user];")
             $null = $db.Query("EXEC sp_addrolemember '$role', '$user';")
+            $null = $db.Query("EXEC sp_addrolemember '$role01', '$user01';")
+            $null = $db.Query("EXEC sp_addrolemember '$role02', '$user01';")
+            $null = $db.Query("EXEC sp_addrolemember '$role02', '$user02';")
+            $null = $db.Query("EXEC sp_addrolemember '$role03', '$user02';")
             $null = $db.Query("GRANT SELECT ON OBJECT::$table TO [$user2];")
         } catch { } # No idea why appveyor can't handle this
     }
@@ -99,8 +120,9 @@ Describe "$commandname Integration Tests" -Tags "IntegrationTests" {
 
     Context "Check if one output file per user was created" {
         $null = Export-DbaUser -SqlInstance $script:instance1 -Database $dbname -Path $outputPath
-        It "Exports two files to the path" {
-            (Get-ChildItem $outputPath).Count | Should Be 2
+        It "Exports files to the path" {
+            $userCount = (Get-DbaDbUser -SqlInstance $script:instance1 -Database $dbname | Where-Object { $_.Name -notin @("dbo", "guest", "sys", "INFORMATION_SCHEMA") } | Measure-Object).Count
+            (Get-ChildItem $outputPath).Count | Should Be $userCount
         }
         It "Exported file name contains username '$user'" {
             Get-ChildItem $outputPath | Where-Object Name -like ('*' + $User + '*') | Should BeTrue
@@ -109,4 +131,34 @@ Describe "$commandname Integration Tests" -Tags "IntegrationTests" {
             Get-ChildItem $outputPath | Where-Object Name -like ('*' + $User2 + '*') | Should BeTrue
         }
     }
+
+    Context "Check if the output scripts were self-contained" {
+        # Clean up the output folder
+        Remove-Item -Path $outputPath -Recurse -ErrorAction SilentlyContinue
+        $null = Export-DbaUser -SqlInstance $script:instance1 -Database $dbname -Path $outputPath
+
+        It "Contains the CREATE ROLE and ALTER ROLE statements for its own roles" {
+            Get-ChildItem $outputPath | Where-Object Name -like ('*' + $user01 + '*') | ForEach-Object {
+                $content = Get-Content -Path $_.FullName -Raw
+                $content | Should BeLike "*CREATE ROLE [[]$role01]*"
+                $content | Should BeLike "*CREATE ROLE [[]$role02]*"
+                $content | Should Not BeLike "*CREATE ROLE [[]$role03]*"
+
+                $content | Should BeLike "*ALTER ROLE [[]$role01] ADD MEMBER [[]$user01]*"
+                $content | Should BeLike "*ALTER ROLE [[]$role02] ADD MEMBER [[]$user01]*"
+                $content | Should Not BeLike "*ALTER ROLE [[]$role03]*"
+            }
+
+            Get-ChildItem $outputPath | Where-Object Name -like ('*' + $user02 + '*') | ForEach-Object {
+                $content = Get-Content -Path $_.FullName -Raw
+                $content | Should BeLike "*CREATE ROLE [[]$role02]*"
+                $content | Should BeLike "*CREATE ROLE [[]$role03]*"
+                $content | Should Not BeLike "*CREATE ROLE [[]$role01]*"
+
+                $content | Should BeLike "*ALTER ROLE [[]$role02] ADD MEMBER [[]$user02]*"
+                $content | Should BeLike "*ALTER ROLE [[]$role03] ADD MEMBER [[]$user02]*"
+                $content | Should Not BeLike "*ALTER ROLE [[]$role01]*"
+            }
+        }
+    }
 }