Fixes missing bulking when adding/removing users to/from CA groups

teamviewer · Jun 25, 2021 · 55b2fe7 · 55b2fe7
1 parent beb20cf
commit 55b2fe7
Show file tree

Hide file tree

Showing 3 changed files with 132 additions and 58 deletions.
diff --git a/README.md b/README.md
@@ -155,48 +155,52 @@ Identification of users is done based on the email addresses.
 If configured, the secondary email addresses of AD users are also taken
 into account for the mapping between AD users and TeamViewer users.
 
-## Changelog	
-
-### [1.3.1]	
-
-- Fixed TeamViewer API calls to use TLS 1.2.	
-
-### [1.3.0]	
-
-- Added synchronization for TeamViewer Conditional Access directory groups.	
-
-### [1.2.2]	
-- Added hint to options that require TeamViewer Tensor license.	
-- Fixed escaping of spaces in script path of scheduled task.	
-- Fixed handling of global catalog names, starting with `GC://`.	
-
-### [1.2.1]	
-- Fixed handling of trailing whitespace in secondary email addresses.	
-- Fixed possible timeouts in update/deactivate user calls to the	
-  TeamViewer Web API on some versions of PowerShell.	
-
-### [1.2.0]	
-- Added configuration field `UseGeneratedPassword` to create user	
-  accounts with a generated password. Such users will receive an email	
-  to reset their password.	
-- Added optional lookup for token owner to avoid accidential	
-  deactivation of the account that owns the configured API token.	
-  This requires additional token permissions.	
-- Version number is now printed to the log file and title bar.	
-- Run in graphical user interface can now be cancelled.	
-- Fixed AD user list to filter-out duplicate users (by email).	
-- Fixed AD groups list UI to strip possible LDAP hostnames.	
-- Fixed sorting of account language list.	
-
-### [1.1.0]	
-- Added option `UseSecondaryEmails` to additionally use the user's	
-  secondary email addresses for the synchronization.	
-- Added configuration field `SsoCustomerId` to create user accounts that	
-  have Single Sign-On already activated.	
-- Added text filtering in the Active Directory groups drop-down menu.	
-  The filter is applied after typing at least 3 characters.	
-- Fixed encoding problem when creating or updating TeamViewer accounts.	
-- Log output now lists changes when updating a user.	
+## Changelog
+
+### [1.3.2]
+
+- Fixed bulking of CA group member requests.
+
+### [1.3.1]
+
+- Fixed TeamViewer API calls to use TLS 1.2.
+
+### [1.3.0]
+
+- Added synchronization for TeamViewer Conditional Access directory groups.
+
+### [1.2.2]
+- Added hint to options that require TeamViewer Tensor license.
+- Fixed escaping of spaces in script path of scheduled task.
+- Fixed handling of global catalog names, starting with `GC://`.
+
+### [1.2.1]
+- Fixed handling of trailing whitespace in secondary email addresses.
+- Fixed possible timeouts in update/deactivate user calls to the
+  TeamViewer Web API on some versions of PowerShell.
+
+### [1.2.0]
+- Added configuration field `UseGeneratedPassword` to create user
+  accounts with a generated password. Such users will receive an email
+  to reset their password.
+- Added optional lookup for token owner to avoid accidential
+  deactivation of the account that owns the configured API token.
+  This requires additional token permissions.
+- Version number is now printed to the log file and title bar.
+- Run in graphical user interface can now be cancelled.
+- Fixed AD user list to filter-out duplicate users (by email).
+- Fixed AD groups list UI to strip possible LDAP hostnames.
+- Fixed sorting of account language list.
+
+### [1.1.0]
+- Added option `UseSecondaryEmails` to additionally use the user's
+  secondary email addresses for the synchronization.
+- Added configuration field `SsoCustomerId` to create user accounts that
+  have Single Sign-On already activated.
+- Added text filtering in the Active Directory groups drop-down menu.
+  The filter is applied after typing at least 3 characters.
+- Fixed encoding problem when creating or updating TeamViewer accounts.
+- Log output now lists changes when updating a user.
 
 ### [1.0.0] - Initial Release
 

diff --git a/TeamViewerADConnector/Internal/Sync.ps1 b/TeamViewerADConnector/Internal/Sync.ps1
@@ -47,6 +47,13 @@ function Format-SyncUpdateUserChangeset {
     }
 }
 
+function Split-Bulk {
+	param([int]$Size)
+	Begin { $bulk = New-Object System.Collections.ArrayList($Size) }
+	Process { $bulk.Add($_) | Out-Null; if ($bulk.Count -ge $Size) { ,$bulk.Clone(); $bulk.Clear() } }
+	End { if ($bulk.Count -gt 0) { ,$bulk } }
+}
+
 function Invoke-SyncPrework($syncContext, $configuration, $progressHandler) {
     # Fetch users from configured AD groups.
     # Map the AD user objects to all their email addresses.
@@ -283,14 +290,19 @@ function Invoke-SyncConditionalAccess($syncContext, $configuration, $progressHan
         }
         Write-SyncLog "Adding $($usersToAdd.Count) users to conditional access group '$($caGroup.name)'"
         if (!$configuration.TestRun -And $usersToAdd.Count -Gt 0) {
-            try {
-                (Add-TeamViewerConditionalAccessGroupUser $configuration.ApiToken $caGroup.id $usersToAdd) | Out-Null
-                $statistics.AddedMembers += $usersToAdd.Count
-            }
-            catch {
-                Write-SyncLog "Failed to add members to conditional access group '$($caGroup.name)': $_"
-                $statistics.Failed += $usersToAdd.Count
-            }
+            $usersToAdd | `
+                Split-Bulk -Size 50 | `
+                ForEach-Object {
+                    $currentUsersToAdd = $_
+                    try {
+                        (Add-TeamViewerConditionalAccessGroupUser $configuration.ApiToken $caGroup.id $currentUsersToAdd) | Out-Null
+                        $statistics.AddedMembers += $currentUsersToAdd.Count
+                    }
+                    catch {
+                        Write-SyncLog "Failed to add members to conditional access group '$($caGroup.name)': $_"
+                        $statistics.Failed += $currentUsersToAdd.Count
+                    }
+                }
         }
         else { $statistics.AddedMembers += $usersToAdd.Count }
 
@@ -305,14 +317,19 @@ function Invoke-SyncConditionalAccess($syncContext, $configuration, $progressHan
         }
         Write-SyncLog "Removing $($usersToRemove.Count) users from conditional access group '$($caGroup.name)'"
         if (!$configuration.TestRun -And $usersToRemove.Count -Gt 0) {
-            try {
-                (Remove-TeamViewerConditionalAccessGroupUser $configuration.ApiToken $caGroup.id $usersToRemove) | Out-Null
-                $statistics.RemovedMembers += $usersToRemove.Count
-            }
-            catch {
-                Write-SyncLog "Failed to remove members from conditional access group '$($caGroup.name)': $_"
-                $statistics.Failed += $usersToRemove.Count
-            }
+            $usersToRemove | `
+                Split-Bulk -Size 50 | `
+                ForEach-Object {
+                    $currentUsersToRemove = $_
+                    try {
+                        (Remove-TeamViewerConditionalAccessGroupUser $configuration.ApiToken $caGroup.id $currentUsersToRemove) | Out-Null
+                        $statistics.RemovedMembers += $currentUsersToRemove.Count
+                    }
+                    catch {
+                        Write-SyncLog "Failed to remove members from conditional access group '$($caGroup.name)': $_"
+                        $statistics.Failed += $currentUsersToRemove.Count
+                    }
+                }
         }
         else { $statistics.RemovedMembers += $usersToRemove.Count }
     }

diff --git a/Tests/TeamViewerADConnector/Internal/Sync.Tests.ps1 b/Tests/TeamViewerADConnector/Internal/Sync.Tests.ps1
@@ -332,6 +332,35 @@ Describe 'Invoke-SyncConditionalAccess' {
             -ParameterFilter { $groupID -Eq 'ca123' -And $userIDs -Eq @('u123') }
     }
 
+    It 'Should create bulks of 50 when adding new users to the conditional access group' {
+        $testTeamViewerUsers = @{}
+        foreach ($count in 1..120) {
+            $testTeamViewerUsers["user$count@example.test"] = [pscustomobject]@{ id = "u$count" }
+        }
+        $testTeamViewerUsers.Count | Should -Be 120
+        $syncContext = @{
+            UsersActiveDirectoryByGroup   = @{
+                'CN=TestGroup' = @(1..120 | ForEach-Object { [pscustomobject]@{ Email = "user$_@example.test" } })
+            }
+            UsersTeamViewerByEmail        = $testTeamViewerUsers
+            UsersConditionalAccessByGroup = @{
+                'ca123' = @()
+            }
+            GroupsConditionalAccess       = @(
+                [pscustomobject]@{ id = 'ca123'; name = 'TestGroup' }
+            )
+        }
+        $configuration = @{
+            ActiveDirectoryGroups = @('CN=TestGroup')
+        }
+        Invoke-SyncConditionalAccess $syncContext $configuration { }
+        Assert-MockCalled Add-TeamViewerConditionalAccessGroupUser -Times 3 -Scope It
+        Assert-MockCalled Add-TeamViewerConditionalAccessGroupUser -Times 2 -Scope It `
+            -ParameterFilter { $groupID -Eq 'ca123' -And $userIDs.Count -Eq 50 }
+        Assert-MockCalled Add-TeamViewerConditionalAccessGroupUser -Times 1 -Scope It `
+            -ParameterFilter { $groupID -Eq 'ca123' -And $userIDs.Count -Eq 20 }
+    }
+
     It 'Should skip existing members of the conditional access group' {
         $syncContext = @{
             UsersActiveDirectoryByGroup   = @{
@@ -383,6 +412,30 @@ Describe 'Invoke-SyncConditionalAccess' {
                 $userIDs.Contains('u123') -And $userIDs.Contains('u456')
         }
     }
+
+    It 'Should create bulks of 50 when removing members from the conditional access group' {
+        $syncContext = @{
+            UsersActiveDirectoryByGroup   = @{
+                'CN=TestGroup' = @()
+            }
+            UsersTeamViewerByEmail        = @{}
+            UsersConditionalAccessByGroup = @{
+                'ca123' = @(1..120 | ForEach-Object { "u@$_" })
+            }
+            GroupsConditionalAccess       = @(
+                [pscustomobject]@{ id = 'ca123'; name = 'TestGroup' }
+            )
+        }
+        $configuration = @{
+            ActiveDirectoryGroups = @('CN=TestGroup')
+        }
+        Invoke-SyncConditionalAccess $syncContext $configuration { }
+        Assert-MockCalled Remove-TeamViewerConditionalAccessGroupUser -Times 3 -Scope It
+        Assert-MockCalled Remove-TeamViewerConditionalAccessGroupUser -Times 2 -Scope It `
+            -ParameterFilter { $userIDs.Count -Eq 50 }
+        Assert-MockCalled Remove-TeamViewerConditionalAccessGroupUser -Times 1 -Scope It `
+            -ParameterFilter { $userIDs.Count -Eq 20 }
+    }
 }
 
 Describe 'Invoke-Sync' {