From 2c541375cfae4fa004f1ecfda9536ee50cd81c3a Mon Sep 17 00:00:00 2001 From: Johan Ljunggren Date: Fri, 2 Aug 2019 15:39:54 +0200 Subject: [PATCH] Changes to ADDomain - BREAKING CHANGE: A new parameter `AllowTrustRecreation` has been added that when set allows a trust to be recreated in scenarios where that is required. This way the user have to opt-in to such destructive action since since it can result in service interruption (issue #421). --- CHANGELOG.md | 4 + .../MSFT_ADDomainTrust.psm1 | 88 ++++++++++++++----- .../MSFT_ADDomainTrust.schema.mof | 1 + .../en-US/MSFT_ADDomainTrust.strings.psd1 | 1 + Tests/Unit/MSFT_ADDomainTrust.Tests.ps1 | 62 +++++++++++++ 5 files changed, 132 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 508f1979a..5556b9cad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -101,6 +101,10 @@ - BREAKING CHANGE: Renamed the parameter `DomainAdministratorCredential` to `Credential` to better indicate that it is possible to impersonate any credential with enough permission to perform the task ([issue #269](https://github.com/PowerShell/ActiveDirectoryDsc/issues/269)). + - BREAKING CHANGE: A new parameter `AllowTrustRecreation` has been added + that when set allows a trust to be recreated in scenarios where that + is required. This way the user have to opt-in to such destructive + action since since it can result in service interruption ([issue #421](https://github.com/PowerShell/ActiveDirectoryDsc/issues/421)). - Updated tests and replaced `Write-Error` with `throw` ([issue #332](https://github.com/PowerShell/ActiveDirectoryDsc/pull/332)). - Added comment-based help ([issue #335](https://github.com/PowerShell/ActiveDirectoryDsc/issues/335)). diff --git a/DSCResources/MSFT_ADDomainTrust/MSFT_ADDomainTrust.psm1 b/DSCResources/MSFT_ADDomainTrust/MSFT_ADDomainTrust.psm1 index 51252549c..400da94ec 100644 --- a/DSCResources/MSFT_ADDomainTrust/MSFT_ADDomainTrust.psm1 +++ b/DSCResources/MSFT_ADDomainTrust/MSFT_ADDomainTrust.psm1 @@ -26,6 +26,10 @@ $script:localizedData = Get-LocalizedData -ResourceName 'MSFT_ADDomainTrust' .PARAMETER TrustDirection Specifies the direction of the trust. + + .PARAMETER AllowTrustRecreation + Specifies if the is allowed to be recreated if required. Default value is + $false. #> function Get-TargetResource { @@ -53,16 +57,21 @@ function Get-TargetResource [Parameter(Mandatory = $true)] [ValidateSet('Bidirectional', 'Inbound', 'Outbound')] [System.String] - $TrustDirection + $TrustDirection, + + [Parameter()] + [System.Boolean] + $AllowTrustRecreation = $false ) # Return a credential object without the password. $cimCredentialInstance = New-CimCredentialInstance -Credential $TargetCredential $returnValue = @{ - SourceDomainName = $SourceDomainName - TargetDomainName = $TargetDomainName - TargetCredential = $cimCredentialInstance + SourceDomainName = $SourceDomainName + TargetDomainName = $TargetDomainName + TargetCredential = $cimCredentialInstance + AllowTrustRecreation = $AllowTrustRecreation } $getTrustTargetAndSourceObject = @{ @@ -127,6 +136,10 @@ function Get-TargetResource .PARAMETER Ensure Specifies whether the computer account is present or absent. Default value is 'Present'. + + .PARAMETER AllowTrustRecreation + Specifies if the is allowed to be recreated if required. Default value is + $false. #> function Set-TargetResource { @@ -158,7 +171,11 @@ function Set-TargetResource [Parameter()] [ValidateSet('Present', 'Absent')] [System.String] - $Ensure = 'Present' + $Ensure = 'Present', + + [Parameter()] + [System.Boolean] + $AllowTrustRecreation = $false ) $getTrustTargetAndSourceObject = @{ @@ -170,15 +187,19 @@ function Set-TargetResource $trustSource, $trustTarget = Get-TrustSourceAndTargetObject @getTrustTargetAndSourceObject - $compareTargetResourceStateResult = Compare-TargetResourceState @PSBoundParameters + # Only pass those properties that should be evaluated. + $compareTargetResourceStateParameters = @{} + $PSBoundParameters + $compareTargetResourceStateParameters.Remove('AllowTrustRecreation') + + $compareTargetResourceStateResult = Compare-TargetResourceState @compareTargetResourceStateParameters # Get all properties that are not in desired state. $propertiesNotInDesiredState = $compareTargetResourceStateResult | - Where-Object -FilterScript { - -not $_.InDesiredState - } + Where-Object -FilterScript { + -not $_.InDesiredState + } - if ($propertiesNotInDesiredState.Where({ $_.ParameterName -eq 'Ensure' })) + if ($propertiesNotInDesiredState.Where( { $_.ParameterName -eq 'Ensure' })) { if ($Ensure -eq 'Present') { @@ -216,7 +237,7 @@ function Set-TargetResource $trustRecreated = $false # Check properties. - $trustTypeProperty = $propertiesNotInDesiredState.Where({ $_.ParameterName -eq 'TrustType' }) + $trustTypeProperty = $propertiesNotInDesiredState.Where( { $_.ParameterName -eq 'TrustType' }) if ($trustTypeProperty) { @@ -229,19 +250,26 @@ function Set-TargetResource ) ) - $trustSource.DeleteTrustRelationship($trustTarget) - $trustSource.CreateTrustRelationship($trustTarget, $TrustDirection) + if ($AllowTrustRecreation) + { + $trustSource.DeleteTrustRelationship($trustTarget) + $trustSource.CreateTrustRelationship($trustTarget, $TrustDirection) - Write-Verbose -Message ( - $script:localizedData.RecreatedTrustType -f @( - $SourceDomainName, - $TargetDomainName, - $TrustType, - $TrustDirection + Write-Verbose -Message ( + $script:localizedData.RecreatedTrustType -f @( + $SourceDomainName, + $TargetDomainName, + $TrustType, + $TrustDirection + ) ) - ) - $trustRecreated = $true + $trustRecreated = $true + } + else + { + throw $script:localizedData.NotOptInToRecreateTrust + } } <# @@ -251,7 +279,7 @@ function Set-TargetResource #> if (-not $trustRecreated) { - if ($propertiesNotInDesiredState.Where({ $_.ParameterName -eq 'TrustDirection' })) + if ($propertiesNotInDesiredState.Where( { $_.ParameterName -eq 'TrustDirection' })) { $trustSource.UpdateTrustRelationship($trustTarget, $TrustDirection) @@ -296,6 +324,10 @@ function Set-TargetResource .PARAMETER Ensure Specifies whether the computer account is present or absent. Default value is 'Present'. + + .PARAMETER AllowTrustRecreation + Specifies if the is allowed to be recreated if required. Default value is + $false. #> function Test-TargetResource { @@ -328,18 +360,26 @@ function Test-TargetResource [Parameter()] [ValidateSet('Present', 'Absent')] [System.String] - $Ensure = 'Present' + $Ensure = 'Present', + + [Parameter()] + [System.Boolean] + $AllowTrustRecreation = $false ) Write-Verbose -Message ( $script:localizedData.TestConfiguration -f $SourceDomainName, $TargetDomainName, $TrustType ) + # Only pass those properties that should be evaluated. + $compareTargetResourceStateParameters = @{} + $PSBoundParameters + $compareTargetResourceStateParameters.Remove('AllowTrustRecreation') + <# This returns array of hashtables which contain the properties ParameterName, Expected, Actual, and InDesiredState. #> - $compareTargetResourceStateResult = Compare-TargetResourceState @PSBoundParameters + $compareTargetResourceStateResult = Compare-TargetResourceState @compareTargetResourceStateParameters if ($false -in $compareTargetResourceStateResult.InDesiredState) { diff --git a/DSCResources/MSFT_ADDomainTrust/MSFT_ADDomainTrust.schema.mof b/DSCResources/MSFT_ADDomainTrust/MSFT_ADDomainTrust.schema.mof index 0e6fd0b7e..1043d91f8 100644 --- a/DSCResources/MSFT_ADDomainTrust/MSFT_ADDomainTrust.schema.mof +++ b/DSCResources/MSFT_ADDomainTrust/MSFT_ADDomainTrust.schema.mof @@ -7,4 +7,5 @@ class MSFT_ADDomainTrust : OMI_BaseResource [Required, Description("Specifies the type of trust. The value 'External' means the context Domain, while the value 'Forest' means the context 'Forest'."), ValueMap{"External","Forest"}, Values{"External","Forest"}] String TrustType; [Required, Description("Specifies the direction of the trust."), ValueMap{"Bidirectional","Inbound","Outbound"}, Values{"Bidirectional","Inbound","Outbound"}] String TrustDirection; [Key, Description("Specifies the name of the Active Directory domain that is requesting the trust.")] String SourceDomainName; + [Write, Description("Specifies if the is allowed to be recreated if required. Default value is $false.")] Boolean AllowTrustRecreation; }; diff --git a/DSCResources/MSFT_ADDomainTrust/en-US/MSFT_ADDomainTrust.strings.psd1 b/DSCResources/MSFT_ADDomainTrust/en-US/MSFT_ADDomainTrust.strings.psd1 index 95360d27c..291635f70 100644 --- a/DSCResources/MSFT_ADDomainTrust/en-US/MSFT_ADDomainTrust.strings.psd1 +++ b/DSCResources/MSFT_ADDomainTrust/en-US/MSFT_ADDomainTrust.strings.psd1 @@ -11,4 +11,5 @@ InDesiredState = The Active Directory trust is in the desired state. NotInDesiredState = The Active Directory trust is not in the desired state. (ADDT0009) NeedToRecreateTrust = The trust type is not in desired state, removing the trust between the domains '{0}' and '{1}' with the context type '{2}' to be able to recreate the trust with the correct context type '{3}'. (ADDT0010) RecreatedTrustType = Recreated the trust between domains '{0}' and '{1}' with the context type '{2}' and direction '{3}'. (ADDT0011) +NotOptInToRecreateTrust = Not opt-in to recreate trust. To opt-in set the parameter AllowTrustRecreation to $true. '@ diff --git a/Tests/Unit/MSFT_ADDomainTrust.Tests.ps1 b/Tests/Unit/MSFT_ADDomainTrust.Tests.ps1 index 42c5f585e..c61e4eeec 100644 --- a/Tests/Unit/MSFT_ADDomainTrust.Tests.ps1 +++ b/Tests/Unit/MSFT_ADDomainTrust.Tests.ps1 @@ -106,6 +106,32 @@ try $getTargetResourceResult = Get-TargetResource @mockGetTargetResourceParameters $getTargetResourceResult.TrustDirection | Should -Be $mockGetTargetResourceParameters.TrustDirection $getTargetResourceResult.TrustType | Should -Be $mockGetTargetResourceParameters.TrustType + $getTargetResourceResult.AllowTrustRecreation | Should -BeFalse + } + + Context 'When the called with the AllowTrustRecreation set to $true' { + BeforeEach { + $mockGetTargetResourceParameters['AllowTrustRecreation'] = $true + } + + It 'Should return the state as present' { + $getTargetResourceResult = Get-TargetResource @mockGetTargetResourceParameters + $getTargetResourceResult.Ensure | Should -Be 'Present' + } + + It 'Should return the same values as passed as parameters' { + $getTargetResourceResult = Get-TargetResource @mockGetTargetResourceParameters + $getTargetResourceResult.SourceDomainName | Should -Be $mockGetTargetResourceParameters.SourceDomainName + $getTargetResourceResult.TargetDomainName | Should -Be $mockGetTargetResourceParameters.TargetDomainName + $getTargetResourceResult.TargetCredential.UserName | Should -Be $mockCredential.UserName + $getTargetResourceResult.AllowTrustRecreation | Should -BeTrue + } + + It 'Should return the correct values for the other properties' { + $getTargetResourceResult = Get-TargetResource @mockGetTargetResourceParameters + $getTargetResourceResult.TrustDirection | Should -Be $mockGetTargetResourceParameters.TrustDirection + $getTargetResourceResult.TrustType | Should -Be $mockGetTargetResourceParameters.TrustType + } } } @@ -157,6 +183,7 @@ try $getTargetResourceResult = Get-TargetResource @mockGetTargetResourceParameters $getTargetResourceResult.TrustDirection | Should -Be $mockGetTargetResourceParameters.TrustDirection $getTargetResourceResult.TrustType | Should -Be $mockGetTargetResourceParameters.TrustType + $getTargetResourceResult.AllowTrustRecreation | Should -BeFalse } } } @@ -206,6 +233,7 @@ try $getTargetResourceResult = Get-TargetResource @mockGetTargetResourceParameters $getTargetResourceResult.TrustDirection | Should -BeNullOrEmpty $getTargetResourceResult.TrustType | Should -BeNullOrEmpty + $getTargetResourceResult.AllowTrustRecreation | Should -BeFalse } } } @@ -791,6 +819,39 @@ try } Context 'When a property of a domain trust is not in desired state' { + Context 'When property TrustType is not in desired state, and not opt-in to recreate trust' { + BeforeAll { + Mock -CommandName Compare-TargetResourceState -MockWith { + return @( + @{ + ParameterName = 'Ensure' + Actual = 'Present' + Expected = 'Present' + InDesiredState = $true + } + @{ + ParameterName = 'TrustType' + Actual = 'Domain' + Expected = 'Forest' + InDesiredState = $false + } + ) + } + } + + BeforeEach { + $setTargetResourceParameters = $mockDefaultParameters.Clone() + $setTargetResourceParameters['TrustType'] = 'Forest' + $setTargetResourceParameters['TrustDirection'] = 'Inbound' + } + + It 'Should not throw and call the correct methods' { + { Set-TargetResource @setTargetResourceParameters } | Should -Throw $script:localizedData.NotOptInToRecreateTrust + + Assert-MockCalled -CommandName Get-TrustSourceAndTargetObject -Exactly -Times 1 -Scope It + } + } + Context 'When both properties TrustType and and TrustDirection is not in desired state' { BeforeAll { Mock -CommandName Compare-TargetResourceState -MockWith { @@ -825,6 +886,7 @@ try $setTargetResourceParameters = $mockDefaultParameters.Clone() $setTargetResourceParameters['TrustType'] = 'Forest' $setTargetResourceParameters['TrustDirection'] = 'Inbound' + $setTargetResourceParameters['AllowTrustRecreation'] = $true } It 'Should not throw and call the correct methods' {