From 20a097c69651d38123de4dd7e4057ed480966293 Mon Sep 17 00:00:00 2001 From: Simon Heather <32168619+X-Guardian@users.noreply.github.com> Date: Tue, 9 Jul 2019 08:07:30 +0100 Subject: [PATCH] xADUser: Fix ChangePasswordAtLogon Property to be only set to True at User Creation (#417) - Changes to xADUser - Fixes ChangePasswordAtLogon Property to be only set to `true` at User Creation (issue #414). --- CHANGELOG.md | 15 +- DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 | 15 +- .../MSFT_xADUser/MSFT_xADUser.schema.mof | 2 +- Tests/Unit/MSFT_xADUser.Tests.ps1 | 128 +++++++++++++++++- 4 files changed, 150 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42074a7ef..fda1d4f83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,17 +3,18 @@ ## Unreleased - Changes to xActiveDirectory - - Added a Requirements section to every DSC resource README with the bullet point stating "Target machine must be running Windows Server 2008 R2 or later" ([issue #399](https://github.com/PowerShell/xActiveDirectory/pull/399)). - - Added 'about_\.help.txt' file to all resources ([issue #404](https://github.com/PowerShell/xActiveDirectory/pull/404)). + - Added a Requirements section to every DSC resource README with the bullet point stating "Target machine must be running Windows Server 2008 R2 or later" ([issue #399](https://github.com/PowerShell/xActiveDirectory/issues/399)). + - Added 'about_\.help.txt' file to all resources ([issue #404](https://github.com/PowerShell/xActiveDirectory/issues/404)). - Changes to xADManagedServiceAccount - - Added a requirement to README stating "Group Managed Service Accounts need at least one Windows Server 2012 Domain Controller" ([issue #399](https://github.com/PowerShell/xActiveDirectory/pull/399)). + - Added a requirement to README stating "Group Managed Service Accounts need at least one Windows Server 2012 Domain Controller" ([issue #399](https://github.com/PowerShell/xActiveDirectory/issues/399)). - Changes to xADComputer - - Fixed the GUID in Example 3-AddComputerAccountSpecificPath_Config. ([issue #410](https://github.com/PowerShell/xActiveDirectory/pull/410)) + - Fixed the GUID in Example 3-AddComputerAccountSpecificPath_Config. ([issue #410](https://github.com/PowerShell/xActiveDirectory/issues/410)). - Changes to xADOrganizationalUnit - - Catch exception when the path property specifies a non-existing path ([issue #408](https://github.com/PowerShell/xActiveDirectory/pull/408)) + - Catch exception when the path property specifies a non-existing path ([issue #408](https://github.com/PowerShell/xActiveDirectory/issues/408)). - Changes to xADUser - - Fixes exception when creating a user with an empty string property ([issue #407](https://github.com/PowerShell/xActiveDirectory/pull/407)). - - Fixes exception when updating `CommonName` and `Path` concurrently ([issue #402](https://github.com/PowerShell/xActiveDirectory/pull/402)). + - Fixes exception when creating a user with an empty string property ([issue #407](https://github.com/PowerShell/xActiveDirectory/issues/407)). + - Fixes exception when updating `CommonName` and `Path` concurrently ([issue #402](https://github.com/PowerShell/xActiveDirectory/issues/402)). + - Fixes ChangePasswordAtLogon Property to be only set to `true` at User Creation ([issue #414](https://github.com/PowerShell/xActiveDirectory/issues/414)). ## 3.0.0.0 diff --git a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 index 04ed7b8b7..dcd078d34 100644 --- a/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 +++ b/DSCResources/MSFT_xADUser/MSFT_xADUser.psm1 @@ -1057,7 +1057,12 @@ function Test-TargetResource $isCompliant = $false } } - # Only check properties that are returned by Get-TargetResource + elseif ($parameter -eq 'ChangePasswordAtLogon' -and $PSBoundParameters.$parameter -eq $true -and $targetResource.Ensure -eq 'Present') + { + # Only process the ChangePasswordAtLogon = $true parameter during new user creation + continue + } + # Only check properties that are returned by Get-TargetResource elseif ($targetResource.ContainsKey($parameter)) { # This check is required to be able to explicitly remove values with an empty string, if required @@ -1450,6 +1455,7 @@ function Set-TargetResource # Add common name, ensure and enabled as they may not be explicitly passed $PSBoundParameters['Ensure'] = $Ensure $PSBoundParameters['Enabled'] = $Enabled + $newADUser = $false if ($Ensure -eq 'Present') { @@ -1485,6 +1491,8 @@ function Set-TargetResource # Now retrieve the newly created user $targetResource = Get-TargetResource @PSBoundParameters + + $newADUser = $true } } @@ -1533,6 +1541,11 @@ function Set-TargetResource Set-ADAccountPassword @adCommonParameters -Reset -NewPassword $Password.Password } } + elseif ($parameter -eq 'ChangePasswordAtLogon' -and $PSBoundParameters.$parameter -eq $true -and $newADUser -eq $false) + { + # Only process the ChangePasswordAtLogon = $true parameter during new user creation + continue + } elseif ($parameter -eq 'Enabled' -and ($PSBoundParameters.$parameter -ne $targetResource.$parameter)) { <# diff --git a/DSCResources/MSFT_xADUser/MSFT_xADUser.schema.mof b/DSCResources/MSFT_xADUser/MSFT_xADUser.schema.mof index 3b2641e1f..3205a844d 100644 --- a/DSCResources/MSFT_xADUser/MSFT_xADUser.schema.mof +++ b/DSCResources/MSFT_xADUser/MSFT_xADUser.schema.mof @@ -45,7 +45,7 @@ class MSFT_xADUser : OMI_BaseResource [Write, Description("Specifies a name in addition to a user's given name and surname, such as the user's middle name. This parameter sets the OtherName property of a user object. The LDAP display name (ldapDisplayName) of this property is middleName.")] String OtherName; [Write, Description("Specifies if the account is enabled (default True)")] Boolean Enabled; [Write, Description("Specifies whether the account password can be changed")] Boolean CannotChangePassword; - [Write, Description("Specifies whether the account password must be changed during the next logon attempt. This cannot be set to true if the PasswordNeverExpires property is also set to true")] Boolean ChangePasswordAtLogon; + [Write, Description("Specifies whether the account password must be changed during the next logon attempt. This will only be set to 'true' once when the user is initially created. This cannot be set to true if the PasswordNeverExpires property is also set to true")] Boolean ChangePasswordAtLogon; [Write, Description("Specifies whether the password of an account can expire")] Boolean PasswordNeverExpires; [Write, Description("Specifies the Active Directory Domain Services instance to use to perform the task.")] String DomainController; [Write, Description("Specifies the user account credentials to use to perform this task"), EmbeddedInstance("MSFT_Credential")] String DomainAdministratorCredential; diff --git a/Tests/Unit/MSFT_xADUser.Tests.ps1 b/Tests/Unit/MSFT_xADUser.Tests.ps1 index b6acca461..5f0427bfa 100644 --- a/Tests/Unit/MSFT_xADUser.Tests.ps1 +++ b/Tests/Unit/MSFT_xADUser.Tests.ps1 @@ -67,7 +67,7 @@ try 'Manager', 'LogonWorkstations', 'Organization', 'OtherName' ) $testBooleanProperties = @( - 'PasswordNeverExpires', 'CannotChangePassword', 'ChangePasswordAtLogon', 'TrustedForDelegation', 'Enabled','AccountNotDelegated', + 'PasswordNeverExpires', 'CannotChangePassword', 'TrustedForDelegation', 'Enabled','AccountNotDelegated', 'AllowReversiblePasswordEncryption', 'CompoundIdentitySupported', 'PasswordNotRequired', 'SmartcardLogonRequired' ) $testArrayProperties = @('ServicePrincipalNames', 'ProxyAddresses') @@ -355,6 +355,80 @@ try } } #end foreach test boolean property + + It "Should pass when ChangePasswordAtLogon is false and matches the AD Account property" { + $testParameter = 'ChangePasswordAtLogon' + $testParameterValue = $false + $testValidPresentParams = $testPresentParams.Clone() + $testValidPresentParams[$testParameter] = $testParameterValue + $validADUser = $testPresentParams.Clone() + Mock -CommandName Get-TargetResource -MockWith { + $validADUser[$testParameter] = $testParameterValue + return $validADUser + } + + Test-TargetResource @testValidPresentParams | Should -Be $true + } + + It "Should fail when ChangePasswordAtLogon is false and does not match the AD Account property" { + $testParameter = 'ChangePasswordAtLogon' + $testParameterValue = $false + $testValidPresentParams = $testPresentParams.Clone() + $testValidPresentParams[$testParameter] = $testParameterValue + $invalidADUser = $testPresentParams.Clone() + Mock -CommandName Get-TargetResource -MockWith { + $invalidADUser[$testParameter] = -not $testParameterValue + return $invalidADUser + } + + Test-TargetResource @testValidPresentParams | Should -Be $false + } + + It "Should pass when ChangePasswordAtLogon is true and matches the AD Account property and the user already exists" { + $testParameter = 'ChangePasswordAtLogon' + $testParameterValue = $true + $testValidPresentParams = $testPresentParams.Clone() + $testValidPresentParams[$testParameter] = $testParameterValue + $validADUser = $testPresentParams.Clone() + $validADUser['Ensure'] = 'Present' + Mock -CommandName Get-TargetResource -MockWith { + $validADUser[$testParameter] = $testParameterValue + return $validADUser + } + + Test-TargetResource @testValidPresentParams | Should -Be $true + } + + It "Should pass when ChangePasswordAtLogon is true and does not match the AD Account property and the user already exists" { + $testParameter = 'ChangePasswordAtLogon' + $testParameterValue = $true + $testValidPresentParams = $testPresentParams.Clone() + $testValidPresentParams[$testParameter] = $testParameterValue + $invalidADUser = $testPresentParams.Clone() + $invalidADUser['Ensure'] = 'Present' + Mock -CommandName Get-TargetResource -MockWith { + $invalidADUser[$testParameter] = -not $testParameterValue + return $invalidADUser + } + + Test-TargetResource @testValidPresentParams | Should -Be $true + } + + It "Should fail when ChangePasswordAtLogon is true and does not match the AD Account property and the user does not exist" { + $testParameter = 'ChangePasswordAtLogon' + $testParameterValue = $true + $testValidPresentParams = $testPresentParams.Clone() + $testValidPresentParams[$testParameter] = $testParameterValue + $invalidADUser = $testPresentParams.Clone() + $invalidADUser['Ensure'] = 'Absent' + Mock -CommandName Get-TargetResource -MockWith { + $invalidADUser[$testParameter] = -not $testParameterValue + return $invalidADUser + } + + Test-TargetResource @testValidPresentParams | Should -Be $false + } + foreach ($testParameter in $testArrayProperties) { It "Passes when user account '$testParameter' matches empty AD account property" { @@ -688,6 +762,58 @@ try Assert-MockCalled -CommandName Set-ADUser -ParameterFilter { $mockBoolParam } -Scope It -Exactly 1 } + It "Should call 'Set-ADUser' with the correct parameter when 'ChangePasswordAtLogon' is true and the user does exist" { + $mockBoolParam = 'ChangePasswordAtLogon' + $newPresentParams = $testPresentParams.Clone() + $newAbsentParams = $testAbsentParams.Clone() + + Mock -CommandName Get-TargetResource -ParameterFilter { $Username -eq $newPresentParams.UserName } ` + -MockWith { $newPresentParams } + Mock -CommandName Set-ADUser + + Set-TargetResource @newPresentParams + + Assert-MockCalled -CommandName Set-ADUser -ParameterFilter { $ChangePasswordAtLogon -eq $true } ` + -Scope It -Exactly 0 + } + + It "Should call 'Set-ADUser' with the correct parameter when 'ChangePasswordAtLogon' is true and the user does not exist" { + $mockNewADUser = @{ + DomainName = 'contoso.com' + UserName = 'NewUser' + ChangePasswordAtLogon = $true + Ensure = 'Present' + } + $mockBoolParam = 'ChangePasswordAtLogon' + $mockPreNewUserParams = $mockNewADUser.Clone() + $mockPreNewUserParams['Ensure'] = 'Absent' + $mockPreNewUserParams[$mockBoolParam] = $false + $mockPostNewUserParams = $mockNewADUser.Clone() + $mockPostNewUserParams[$mockBoolParam] = $false + + Mock -CommandName New-ADUser -MockWith { + $script:mockNewADUserWasCalled = $true + } + Mock -CommandName Get-TargetResource ` + -MockWith { + if (-not $script:mockNewADUserWasCalled) + { + $mockPreNewUserParams + } + else + { + $mockPostNewUserParams + } + } + Mock -CommandName Set-ADUser + + Set-TargetResource @mockNewADUser + + Assert-MockCalled -CommandName Get-TargetResource -ParameterFilter { $Username -eq $mockPreNewUserParams.UserName } + Assert-MockCalled -CommandName Set-ADUser -ParameterFilter { $ChangePasswordAtLogon -eq $true } ` + -Scope It -Exactly 1 + } + Context 'When RestoreFromRecycleBin is used' { BeforeAll { Mock -CommandName Get-TargetResource -MockWith {