Open ShawnHardwick opened 3 years ago
Merging #161 (620a4ef) into master (44acc25) will increase coverage by
0%
. The diff coverage is66%
.
@@ Coverage Diff @@
## master #161 +/- ##
=====================================
Coverage 89% 89%
=====================================
Files 5 5
Lines 577 579 +2
=====================================
+ Hits 517 520 +3
+ Misses 60 59 -1
I believe this PR is ready for review. It is my understanding that the currently failing CI tests are not related to my PR.
Is there anything missing that I need to address to have this reviewed?
Can this be reviewed for merge? I would like to utilize this. Thanks!
I've just kicked another build to check, but please make sure all the test pass for starter. I think some High Quality Resource Modules were not passing.
Ok they're passing. @bcwilhite could you check this PR please and review the proposal please?
Still looking to get this reviewed. :)
Can this be reviewed please?
Bump.... Would really like to see this merged.
Bump for release.
Still looking for someone to review this lol. We've forked this internally, but would see benefit in having this merged.
Will bump this in the #DSC channel of the PowerShell slack/discord
Hi there. Any updates on this? Facing the same problem.
We're hitting the same problem also, would be good to get a fix
Pull Request (PR) description
Do not attempt to translate identities returned by local security policy when setting the resource. If the identity returned is an unresolvable SID (usually due to deleted/orphaned AD objects), then the resource will throw a translation error.
I also thought of changing the
ConvertTo-NTAccount
function to never throw but instead log verbosely for all scope types, but felt that the implementation in this PR was more precise.https://github.com/dsccommunity/SecurityPolicyDsc/blob/b29bdf824d91f72375de7851e20c8adf526501cd/source/Modules/SecurityPolicyResourceHelper/SecurityPolicyResourceHelper.psm1#L369-L377
The resource will still translate any identities given to the resource by the user.
Neither secedit nor Windows care about the presence of unresolved SIDs in a security policy. During a configure operation (secedit.exe /configure), secedit will happily accept either style in the .inf file. It will also accept and de-duplicate security principals that are specified as an account name and SID in the same line item under the [Privileges] section.
First attempt at using Pester unit testing before. I don't have a way to test locally due to permission restrictions.
This Pull Request (PR) fixes the following issues
Task list
This change is![Reviewable](https://reviewable.io/review_button.svg)