Fix to issue 116: Test-TargetResource throws System.InvalidOperationException when defined user lacks permissions

[djwork]

Fix to issue 116: Test-TargetResource throws System.InvalidOperationException when defined user lacks permissions

• Observations
  • Used the User DSCR to create a local user 'NoGroupTest' on the target node, when the example configuration is initially applied there are no errors in the console or Microsoft-Windows-DSC/Operational.
  • However once the LCM auto applies the current config (or a user runs: Start-DscConfiguration -UseExisting -Force -Wait) the LCM will report exceptions thrown by User: Test-TargetResource
  • Traced the exception down to the helper function MSFT_UserResource\Test-UserPasswordOnFullSku specifically the call to ValidateCredentials method of the class System.DirectoryServices.AccountManagement.PrincipalContext
  • Tried work arounds like adding the test user to the local admin group but that made no difference
  • Did some research and found the above method has some known issues and the recommedation is call the WIN32 function LogonUser in advapi32.dll
  • This pull request contains the modification to the helper function MSFT_UserResource\Test-UserPasswordOnFullSku to use LogonUser
  • The above change appears to work for users even if they are not a member of any local group

---

This change is 

[codecov-io]

Codecov Report

Merging #117 into dev will decrease coverage by <1%.
The diff coverage is 0%.

@@         Coverage Diff         @@
##            dev   #117   +/-   ##
===================================
- Coverage    83%    83%   -1%     
===================================
  Files        19     19           
  Lines      2760   2770   +10     
  Branches      4      4           
===================================
  Hits       2305   2305           
- Misses      451    461   +10     
  Partials      4      4

[stale]

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

[PlagueHO]

Hi @djwork - thanks for getting this one in!

[PlagueHO]

Is it possible to get some unit tests over this code to increase the test coverage to meet the codecov targets?

We'll also need to add an entry in the #Unreleased section of the README.MD.

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @djwork and @kwirkykat)

---

appveyor.yml, line 4 at r1 (raw file):

#      environment configuration  #
#---------------------------------#
version: 2.9.{build}.0

Could you revert this change? It is actually set by the DSC team during the release process.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1403 at r1 (raw file):

                                        ref IntPtr phToken );
'@
    

Could you trim this whitespace?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1409 at r1 (raw file):

public static extern bool CloseHandle( IntPtr handle );
'@
    

Could you trim this whitespace?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1410 at r1 (raw file):

'@
    
    $AdvApi32 = Add-Type -MemberDefinition $logonUserSignature -Name "AdvApi32" -Namespace "PsInvoke.NativeMethods" -PassThru

Can you use lower case first letter for local variables?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1411 at r1 (raw file):

    
    $AdvApi32 = Add-Type -MemberDefinition $logonUserSignature -Name "AdvApi32" -Namespace "PsInvoke.NativeMethods" -PassThru
    $Kernel32 = Add-Type -MemberDefinition $closeHandleSignature -Name "Kernel32" -Namespace "PsInvoke.NativeMethods" -PassThru

Can you use lower case first letter for local variables?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1412 at r1 (raw file):

    $AdvApi32 = Add-Type -MemberDefinition $logonUserSignature -Name "AdvApi32" -Namespace "PsInvoke.NativeMethods" -PassThru
    $Kernel32 = Add-Type -MemberDefinition $closeHandleSignature -Name "Kernel32" -Namespace "PsInvoke.NativeMethods" -PassThru
    [Reflection.Assembly]::LoadWithPartialName("System.Security") | Out-Null

Can you use $null = ... instead of piping to | Out-Null

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1414 at r1 (raw file):

    [Reflection.Assembly]::LoadWithPartialName("System.Security") | Out-Null

    $Logon32ProviderDefault = 0

Can you use lower case first letter for local variables?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1414 at r1 (raw file):

    [Reflection.Assembly]::LoadWithPartialName("System.Security") | Out-Null

    $Logon32ProviderDefault = 0

Could you add a comment describing the behavior of these magic numbers? E.g.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1415 at r1 (raw file):

    $Logon32ProviderDefault = 0
    $Logon32LogonInteractive = 2

Can you use lower case first letter for local variables?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1418 at r1 (raw file):

    $tokenHandle = [IntPtr]::Zero
    $success = $false
    $DomainName = $null

Can you use lower case first letter for local variables?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1424 at r1 (raw file):

    if (-not $success)
    {        

Can you trim this whitespace?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1426 at r1 (raw file):

    {        
        return $false
    } else {     

Can you move { and } to be on different lines. E.g.

}
else
{

Can you trim this whitespace?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1427 at r1 (raw file):

        return $false
    } else {     
        $Kernel32::CloseHandle( $tokenHandle ) | Out-Null

Can you use $null = ... instead of piping to | Out-Null

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1428 at r1 (raw file):

    } else {     
        $Kernel32::CloseHandle( $tokenHandle ) | Out-Null
        return $True

Can you use lower case 'T' in $true.

[stale]

Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again.

[djwork]

Reviewable status: 0 of 1 files reviewed, 14 unresolved discussions (waiting on @kwirkykat and @PlagueHO)

---

appveyor.yml, line 4 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you revert this change? It is actually set by the DSC team during the release process.

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1403 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you trim this whitespace?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1409 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you trim this whitespace?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1410 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use lower case first letter for local variables?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1411 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use lower case first letter for local variables?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1412 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use $null = ... instead of piping to | Out-Null

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1414 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use lower case first letter for local variables?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1414 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Could you add a comment describing the behavior of these magic numbers? E.g.

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1415 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use lower case first letter for local variables?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1418 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use lower case first letter for local variables?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1424 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you trim this whitespace?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1426 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you move { and } to be on different lines. E.g.

}
else
{

Can you trim this whitespace?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1427 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use $null = ... instead of piping to | Out-Null

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1428 at r1 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use lower case 'T' in $true.

Done.

[PlagueHO]

Can you also add an entry in the # Unreleased section of the README.MD?

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @djwork and @kwirkykat)

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1405 at r2 (raw file):

'@

    $advApi32 = Add-Type -MemberDefinition $logonUserSignature -Name "AdvApi32" -Namespace "PsInvoke.NativeMethods" -PassThru

Can you use single quotes around the strings here?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1406 at r2 (raw file):

    $advApi32 = Add-Type -MemberDefinition $logonUserSignature -Name "AdvApi32" -Namespace "PsInvoke.NativeMethods" -PassThru
    $kernel32 = Add-Type -MemberDefinition $closeHandleSignature -Name "Kernel32" -Namespace "PsInvoke.NativeMethods" -PassThru

Can you use single quotes around the strings here?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1407 at r2 (raw file):

    $advApi32 = Add-Type -MemberDefinition $logonUserSignature -Name "AdvApi32" -Namespace "PsInvoke.NativeMethods" -PassThru
    $kernel32 = Add-Type -MemberDefinition $closeHandleSignature -Name "Kernel32" -Namespace "PsInvoke.NativeMethods" -PassThru
    $null = [Reflection.Assembly]::LoadWithPartialName("System.Security")

Can you use single quotes around this string?

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1409 at r2 (raw file):

    $null = [Reflection.Assembly]::LoadWithPartialName("System.Security")

    #LOGON32_PROVIDER_DEFAULT, Use the standard logon provider for the system

Can you use Block comment for these? E.g.

<#
    Comment
    Comment
#>

And below

[djwork]

Fix to issue 116: Test-TargetResource throws System.InvalidOperationException when defined user lacks permissions
Done, did best to copy existing readme entry format

Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @kwirkykat and @PlagueHO)

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1405 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use single quotes around the strings here?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1406 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use single quotes around the strings here?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1407 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use single quotes around this string?

Done.

---

DscResources/MSFT_UserResource/MSFT_UserResource.psm1, line 1409 at r2 (raw file):

Previously, PlagueHO (Daniel Scott-Raynsford) wrote…

Can you use Block comment for these? E.g.

<#
    Comment
    Comment
#>

And below

Done.