xADObjectPermissionEntry: Fix failure when applied in the same configuration as xADDomain

CHANGELOG.md

@@ -47,6 +47,9 @@
   - Suppressing the Script Analyzer rule `PSAvoidGlobalVars` since the
     resource is using the `$global:DSCMachineStatus` variable to trigger
     a reboot.
+- Changes to xADObjectPermissionEntry
+  - Fix failure when applied in the same configuration as xADDomain
+  - Localize and Improve verbose messaging
 
 ## 2.26.0.0
 

DSCResources/MSFT_xADCommon/MSFT_xADCommon.psm1

@@ -39,6 +39,8 @@ data localizedString
         RecycleBinRestoreSuccessful      = Successfully restored object {0} ({1}) from the recycle bin. (ADCOMMON0029)
         AddingGroupMember                = Adding member '{0}' from domain '{1}' to AD group '{2}'. (ADCOMMON0030)
         PropertyMapArrayIsWrongType      = An object in the property map array is not of the type [System.Collections.Hashtable]. (ADCOMMON0031)
+        CreatingNewADPSDrive             = Creating new AD: PSDrive. (ADCOMMON0032)
+        CreatingNewADPSDriveError        = Error creating AD: PS Drive. (ADCOMMON0033)
 '@
 }
 
@@ -1309,3 +1311,42 @@ function Test-DscPropertyState
 
     return $returnValue
 }
+
+<#
+    .SYNOPSIS
+        Asserts if the AD PS Drive has been created, and creates one if not.
+
+    .PARAMETER Root
+        Specifies the AD path to which the drive is mapped.
+
+    .NOTES
+        Throws an exception if the PS Drive cannot be created.
+#>
+function Assert-ADPSDrive
+{
+    [CmdletBinding()]
+    param
+    (
+        [Parameter()]
+        [String]
+        $Root = '//RootDSE/'
+    )
+
+    Assert-Module -ModuleName 'ActiveDirectory' -ImportModule
+
+    $activeDirectoryPSDrive = Get-PSDrive -Name AD -ErrorAction SilentlyContinue
+
+    if ($null -eq $activeDirectoryPSDrive)
+    {
+        Write-Verbose -Message $script:localizedString.CreatingNewADPSDrive
+        try
+        {
+            New-PSDrive -Name AD -PSProvider 'ActiveDirectory' -Root $Root -Scope Script -ErrorAction Stop | Out-Null
+        }
+        catch
+        {
+            $errorMessage = $script:localizedString.CreatingNewADPSDriveError
+            New-InvalidOperationException -Message $errorMessage -ErrorRecord $_
+        }
+    }
+}

DSCResources/MSFT_xADObjectPermissionEntry/MSFT_xADObjectPermissionEntry.psm1

@@ -1,7 +1,14 @@
+# Import the common AD functions
+$adCommonFunctions = Join-Path `
+    -Path (Split-Path -Path $PSScriptRoot -Parent) `
+    -ChildPath '\MSFT_xADCommon\MSFT_xADCommon.psm1'
+Import-Module -Name $adCommonFunctions
+
+$script:localizedData = Get-LocalizedData -ResourceName 'MSFT_xADObjectPermissionEntry'
 
 <#
     .SYNOPSIS
-        Get the current state of the object access entry.
+        Get the current state of the object permission entry.
 
     .PARAMETER Path
         Active Directory path of the target object to add or remove the
@@ -57,8 +64,7 @@ function Get-TargetResource
         $InheritedObjectType
     )
 
-    # Import the Active Directory module for the AD: drive
-    Import-Module -Name 'ActiveDirectory' -Verbose:$false
+    Assert-ADPSDrive
 
     # Return object, by default representing an absent ace
     $returnValue = @{
@@ -89,26 +95,24 @@ function Get-TargetResource
                 $access.InheritanceType -eq $ActiveDirectorySecurityInheritance -and
                 $access.InheritedObjectType.Guid -eq $InheritedObjectType)
             {
-                Write-Verbose "Target ace has been found"
+                Write-Verbose -Message ($script:localizedData.ObjectPermissionEntryFound -f $Path)
 
                 $returnValue['Ensure'] = 'Present'
                 $returnValue['ActiveDirectoryRights'] = [String[]] $access.ActiveDirectoryRights.ToString().Split(',').ForEach({ $_.Trim() })
 
                 return $returnValue
             }
-            else
-            {
-                Write-Verbose "Target ace has not been found"
-            }
         }
     }
 
+    Write-Verbose -Message ($script:localizedData.ObjectPermissionEntryNotFound -f $Path)
+
     return $returnValue
 }
 
 <#
     .SYNOPSIS
-        Add or remove the object access entry.
+        Add or remove the object permission entry.
 
     .PARAMETER Ensure
         Indicates if the access will be added (Present) or will be removed
@@ -183,15 +187,14 @@ function Set-TargetResource
         $InheritedObjectType
     )
 
-    # Import the Active Directory module for the AD: drive
-    Import-Module -Name 'ActiveDirectory' -Verbose:$false
+    Assert-ADPSDrive
 
     # Get the current acl
     $acl = Get-Acl -Path "AD:$Path"
 
     if ($Ensure -eq 'Present')
     {
-        Write-Verbose "Add access rule to object $Path"
+        Write-Verbose -Message ($script:localizedData.AddingObjectPermissionEntry -f $Path)
 
         $ntAccount = New-Object -TypeName 'System.Security.Principal.NTAccount' -ArgumentList $IdentityReference
 
@@ -220,7 +223,7 @@ function Set-TargetResource
                     $access.InheritanceType -eq $ActiveDirectorySecurityInheritance -and
                     $access.InheritedObjectType.Guid -eq $InheritedObjectType)
                 {
-                    Write-Verbose "Remove access rule on object $Path"
+                    Write-Verbose -Message ($script:localizedData.RemovingObjectPermissionEntry -f $Path)
 
                     $acl.RemoveAccessRule($access)
                 }
@@ -234,7 +237,7 @@ function Set-TargetResource
 
 <#
     .SYNOPSIS
-        Test the object access entry.
+        Test the object permission entry.
 
     .PARAMETER Ensure
         Indicates if the access will be added (Present) or will be removed
@@ -333,5 +336,14 @@ function Test-TargetResource
         $returnValue = $returnValue -and $currentActiveDirectoryRights -eq $desiredActiveDirectoryRights
     }
 
+    if ($returnValue)
+    {
+        Write-Verbose -Message ($script:localizedData.ObjectPermissionEntryInDesiredState -f $Path)
+    }
+    else
+    {
+        Write-Verbose -Message ($script:localizedData.ObjectPermissionEntryNotInDesiredState -f $Path)
+    }
+
     return $returnValue
 }

DSCResources/MSFT_xADObjectPermissionEntry/en-US/MSFT_xADObjectPermissionEntry.strings.psd1

@@ -0,0 +1,10 @@
+
+# culture='en-US'
+ConvertFrom-StringData @'
+    ObjectPermissionEntryFound             = Object permission entry found on object '{0}'. (OPE0001)
+    ObjectPermissionEntryNotFound          = Object permission entry not found on object '{0}'. (OPE0002)
+    AddingObjectPermissionEntry            = Adding object permission entry to object '{0}'. (OPE0003)
+    RemovingObjectPermissionEntry          = Removing object permission entry from object '{0}'. (OPE0004)
+    ObjectPermissionEntryInDesiredState    = Object permission entry on object '{0}' is in the desired state. (OPE0005)
+    ObjectPermissionEntryNotInDesiredState = Object permission entry on object '{0}' is not in the desired state. (OPE0006)
+'@

Tests/Unit/MSFT_xADCommon.Tests.ps1

@@ -8,7 +8,7 @@ $Global:DSCResourceName    = 'MSFT_xADCommon' # Example MSFT_xFirewall
 [String] $moduleRoot = Split-Path -Parent (Split-Path -Parent (Split-Path -Parent $Script:MyInvocation.MyCommand.Path))
 Write-Host $moduleRoot -ForegroundColor Green;
 if ( (-not (Test-Path -Path (Join-Path -Path $moduleRoot -ChildPath 'DSCResource.Tests'))) -or `
-     (-not (Test-Path -Path (Join-Path -Path $moduleRoot -ChildPath 'DSCResource.Tests\TestHelper.psm1'))) )
+    (-not (Test-Path -Path (Join-Path -Path $moduleRoot -ChildPath 'DSCResource.Tests\TestHelper.psm1'))) )
 {
     & git @('clone','https://github.com/PowerShell/DscResource.Tests.git',(Join-Path -Path $moduleRoot -ChildPath '\DSCResource.Tests\'))
 }
@@ -1445,6 +1445,74 @@ try
                 }
             }
         }
+        #region Function Assert-ADPSDrive
+        Describe "$($Global:DSCResourceName)\Assert-ADPSDrive" {
+            Mock -CommandName Assert-Module
+
+            Context 'When the AD PS Drive does not exist and the New-PSDrive function is successful' {
+                Mock -CommandName Get-PSDrive -MockWith { $null }
+                Mock -CommandName New-PSDrive
+
+                It 'Should not throw' {
+                    { Assert-ADPSDrive } | Should -Not -Throw
+                }
+
+                It 'Should have called Assert-Module' {
+                    Assert-MockCalled -CommandName Assert-Module -Exactly -Times 1 -Scope Context
+                }
+
+                It 'Should have called Get-PSDrive only once' {
+                    Assert-MockCalled -CommandName Get-PSDrive -Exactly -Times 1 -Scope Context
+                }
+
+                It 'Should have called New-PSDrive only once' {
+                    Assert-MockCalled -CommandName New-PSDrive -Exactly -Times 1 -Scope Context
+                }
+            }
+
+            Context 'When the AD PS Drive does not exist and the New-PSDrive function is not successful' {
+                Mock -CommandName Get-PSDrive -MockWith { $null }
+                Mock -CommandName New-PSDrive -MockWith { throw }
+
+                It 'Should throw the correct error' {
+                    { Assert-ADPSDrive } | Should -Throw $script:localizedString.CreatingNewADPSDriveError
+                }
+
+                It 'Should call Assert-Module' {
+                    Assert-MockCalled -CommandName Assert-Module -Exactly -Times 1 -Scope Context
+                }
+
+                It 'Should call Get-PSDrive once' {
+                    Assert-MockCalled -CommandName Get-PSDrive -Exactly -Times 1 -Scope Context
+                }
+
+                It 'Should call New-PSDrive once' {
+                    Assert-MockCalled -CommandName New-PSDrive -Exactly -Times 1 -Scope Context
+                }
+            }
+
+            Context 'When the AD PS Drive already exists' {
+                Mock -CommandName Get-PSDrive -MockWith { New-MockObject -Type System.Management.Automation.PSDriveInfo }
+                Mock -CommandName New-PSDrive
+
+                It 'Should not throw' {
+                  { Assert-ADPSDrive } | Should -Not -Throw
+                }
+
+                It 'Should call Assert-Module only once' {
+                    Assert-MockCalled -CommandName Assert-Module -Exactly -Times 1 -Scope Context
+                }
+
+                It 'Should call Get-PSDrive only once' {
+                    Assert-MockCalled -CommandName Get-PSDrive -Exactly -Times 1 -Scope Context
+                }
+
+                It 'Should not call New-PSDrive' {
+                    Assert-MockCalled -CommandName New-PSDrive -Exactly -Times 0 -Scope Context
+                }
+            }
+        }
+        #endregion
     }
     #endregion
 }

Tests/Unit/MSFT_xADObjectPermissionEntry.Tests.ps1

@@ -85,12 +85,17 @@ try
         #region Function Get-TargetResource
         Describe "$($Global:DSCResourceName)\Get-TargetResource" {
 
-            Mock -CommandName 'Import-Module' -ParameterFilter { $Name -eq 'ActiveDirectory' } -MockWith { }
+            Mock -CommandName 'Assert-ADPSDrive' -MockWith { }
 
             Context 'When the desired ace is present' {
 
                 Mock -CommandName 'Get-Acl' -MockWith $mockGetAclPresent
 
+                It 'Should call "Assert-ADPSDrive" to check AD PS Drive is created' {
+                    $targetResource = Get-TargetResource @testDefaultParameters -Verbose
+                    Assert-MockCalled -CommandName Assert-ADPSDrive -Scope It -Exactly -Times 1
+                }
+
                 It 'Should return a "System.Collections.Hashtable" object type' {
                     # Act
                     $targetResource = Get-TargetResource @testDefaultParameters -Verbose
@@ -119,6 +124,11 @@ try
 
                 Mock -CommandName 'Get-Acl' -MockWith $mockGetAclAbsent
 
+                It 'Should call "Assert-ADPSDrive" to check AD PS Drive is created' {
+                    $targetResource = Get-TargetResource @testDefaultParameters -Verbose
+                    Assert-MockCalled -CommandName Assert-ADPSDrive -Scope It -Exactly -Times 1
+                }
+
                 It 'Should return a valid result if the ace is absent' {
                     # Act
                     $targetResource = Get-TargetResource @testDefaultParameters -Verbose
@@ -140,9 +150,9 @@ try
         #region Function Test-TargetResource
         Describe "$($Global:DSCResourceName)\Test-TargetResource" {
 
-            Mock -CommandName 'Import-Module' -ParameterFilter { $Name -eq 'ActiveDirectory' } -MockWith { }
+            Mock -CommandName 'Assert-ADPSDrive' { }
 
-            Context 'When he desired ace is present' {
+            Context 'When the desired ace is present' {
 
                 Mock -CommandName 'Get-Acl' -MockWith $mockGetAclPresent
 
@@ -197,13 +207,18 @@ try
         #region Function Set-TargetResource
         Describe "$($Global:DSCResourceName)\Set-TargetResource" {
 
-            Mock -CommandName 'Import-Module' -ParameterFilter { $Name -eq 'ActiveDirectory' } -MockWith { }
+            Mock -CommandName 'Assert-ADPSDrive' -MockWith { }
 
             Context 'When the desired ace is present' {
 
                 Mock -CommandName 'Get-Acl' -MockWith $mockGetAclPresent
                 Mock -CommandName 'Set-Acl' -MockWith { } -Verifiable
 
+                It 'Should call "Assert-ADPSDrive" to check AD PS Drive is created' {
+                    $targetResource = Get-TargetResource @testDefaultParameters -Verbose
+                    Assert-MockCalled -CommandName Assert-ADPSDrive -Scope It -Exactly -Times 1
+                }
+
                 It 'Should remove the ace from the existing acl' {
                     # Act
                     Set-TargetResource @testDefaultParameters @testAbsentParameters
@@ -218,6 +233,11 @@ try
                 Mock -CommandName 'Get-Acl' -MockWith $mockGetAclAbsent
                 Mock -CommandName 'Set-Acl' -MockWith { } -Verifiable
 
+                It 'Should call "Assert-ADPSDrive" to check AD PS Drive is created' {
+                    $targetResource = Get-TargetResource @testDefaultParameters -Verbose
+                    Assert-MockCalled -CommandName Assert-ADPSDrive -Scope It -Exactly -Times 1
+                }
+
                 It 'Should add the ace to the existing acl' {
                     # Act
                     Set-TargetResource @testDefaultParameters @testPresentParameters