NTFSAccessControl Refactoring and fix for bug #46 - NTFSAccessControlEntry - False Positive Test Bug #47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## Dev #47 +/- ##
=================================
- Coverage 54% 52% -2%
=================================
Files 5 5
Lines 999 965 -34
=================================
- Hits 545 511 -34
Misses 454 454 |
chasewilson
approved these changes
Feb 27, 2019
mcollera
added a commit
that referenced
this pull request
Sep 29, 2022
* Active directory access entry (#28) * initial ActiveDirectoryAccessEntry resource * updates to ActiveDirectoryAccessEntry resource * ActiveDirectoryAccessEntry unit test; resource fixes * updated readme; added example; mof fixes * version rev * AuditRule fixes * Updated issue with ACLRules not always being an array when trying to add additional objects. Updated issue where Expected.Rules might only be a single object while trying to call a Where extension method. (#31) * Rights guid (#32) * Updated ActiveDirectoryAccessEntry example with a valid ADRights value Refactored Get-SchemaGuidId helper function to Get-DelegationRightsGuid so it returns schemaGuids and rightsGuids * typo corrections * Update Get-SchemaObjectName to resolve SchemaGuids and RightsGuids * Added $guidmap to Get-SchemaObjectName * Added $rootDse to Get-SchemaObjectName * Changes RegistryAccessEntry to correctly remove specific ACEs from ACLs and gracefully handle the App Packages Principal, Issues #37 and #38 (#39) * Added ConvertTo-SidIdentityReg... funct. to addr app packages transation * Mod. Rule/Expected foreach to correct rule input for ConvertTo-SidId... * modified RemoveAccessRule to RemoveAccessRuleSpecific to addr. issue #38 * added Set-RegistryRightsAclAllAppPackages function * added test for Set-RegistryRightsAclAllAppPackages function. * updated test to hanlde deny scenario * updated formatting and defined output type for New-TempAclItem * Update ReadMe to include new version and changes. (#42) * Added ConvertTo-SidIdentityReg... funct. to addr app packages transation * Mod. Rule/Expected foreach to correct rule input for ConvertTo-SidId... * modified RemoveAccessRule to RemoveAccessRuleSpecific to addr. issue #38 * added Set-RegistryRightsAclAllAppPackages function * added test for Set-RegistryRightsAclAllAppPackages function. * updated test to hanlde deny scenario * updated formatting and defined output type for New-TempAclItem * updated readme.md * NTFSAccessControlEntry Resource Bug Fix (#45) * Added ConvertTo-SidIdentityReg... funct. to addr app packages transation * Mod. Rule/Expected foreach to correct rule input for ConvertTo-SidId... * modified RemoveAccessRule to RemoveAccessRuleSpecific to addr. issue #38 * added Set-RegistryRightsAclAllAppPackages function * added test for Set-RegistryRightsAclAllAppPackages function. * updated test to hanlde deny scenario * updated formatting and defined output type for New-TempAclItem * updated readme.md * updated Test/Set to use Get/SetAccessControl Methods, vs. Get/Set-Acl * updated readme.md with bug fix information * NTFSAccessControl Refactoring and fix for bug #46 - NTFSAccessControlEntry - False Positive Test Bug (#47) * Added ConvertTo-SidIdentityReg... funct. to addr app packages transation * Mod. Rule/Expected foreach to correct rule input for ConvertTo-SidId... * modified RemoveAccessRule to RemoveAccessRuleSpecific to addr. issue #38 * added Set-RegistryRightsAclAllAppPackages function * added test for Set-RegistryRightsAclAllAppPackages function. * updated test to hanlde deny scenario * updated formatting and defined output type for New-TempAclItem * updated readme.md * updated Test/Set to use Get/SetAccessControl Methods, vs. Get/Set-Acl * updated readme.md with bug fix information * pre-test run, new feature, w/force wipe acl * updated NTFSAccessEntry.psm1 to clear existing currentacl * refactoring changes. * bug fix #46 and code refactoring with added Write-CustomVerbose function * updated README.md with bug fix information * updated fs rights logic to ensure test runs when permissions are less. * Update ModuleVersion in Module Manifest psd1 (#48) * Added ConvertTo-SidIdentityReg... funct. to addr app packages transation * Mod. Rule/Expected foreach to correct rule input for ConvertTo-SidId... * modified RemoveAccessRule to RemoveAccessRuleSpecific to addr. issue #38 * added Set-RegistryRightsAclAllAppPackages function * added test for Set-RegistryRightsAclAllAppPackages function. * updated test to hanlde deny scenario * updated formatting and defined output type for New-TempAclItem * updated readme.md * updated Test/Set to use Get/SetAccessControl Methods, vs. Get/Set-Acl * updated readme.md with bug fix information * pre-test run, new feature, w/force wipe acl * updated NTFSAccessEntry.psm1 to clear existing currentacl * refactoring changes. * bug fix #46 and code refactoring with added Write-CustomVerbose function * updated README.md with bug fix information * updated fs rights logic to ensure test runs when permissions are less. * incremented the ModuleVersion in module manifest psd1. * Fix for Feature Request #49 - ObjectType Parameter in ActiveDirectoryAuditRuleEntry (#50) * refactor work * daily commit for refactor work * add support for objecttype and central localization text * updated code to be in line with style guide lines. * updated NTFSAccessEntry with import localization based on PSUICulture * updated/refactor tests to handle objecttype parameter * Finish Get Method * Test method first version * 1st version Set method * Test Set working Get broken * Everything works. Needs tests * Started unit tests * Unit tests done * Add integration tests but not verified * Correct typo in FileSystemAuidtRuleEntry unit test * Updater readme and examples FileSystemAuditRuleEntry * Updated schema files Moved base class to top of file to pass PSScriptAnalyzer rule * Update tests * Update testHelper path * Resolve PR descrepancies * Updated module version * put comments on get/test/set functions * Style updates * style corrections * AccessControlDSC v1.4.0.0 is throwing error while trying to set permission to a folder for the group "ALL APPLICATION PACKAGES" and "ALL RESTRICTED APPLICATION PACKAGES" (#59) * workaround for Win32 API bug App Package * added new line to the EoF * updated appveyor.yml * updated appveyor.yml * updated appveyor.yml to use Pester 4.10.1 * update tests to satisfy code coverage * updated module manifest with symantic versioning * fixed NTFS PowerShell v7 compat * Capabilitysids (#65) * Merge Dev into Master Build 1.1.0.0 (#33) * Active directory access entry (#28) * initial ActiveDirectoryAccessEntry resource * updates to ActiveDirectoryAccessEntry resource * ActiveDirectoryAccessEntry unit test; resource fixes * updated readme; added example; mof fixes * version rev * AuditRule fixes * Updated issue with ACLRules not always being an array when trying to add additional objects. Updated issue where Expected.Rules might only be a single object while trying to call a Where extension method. (#31) * Rights guid (#32) * Updated ActiveDirectoryAccessEntry example with a valid ADRights value Refactored Get-SchemaGuidId helper function to Get-DelegationRightsGuid so it returns schemaGuids and rightsGuids * typo corrections * Update Get-SchemaObjectName to resolve SchemaGuids and RightsGuids * Added $guidmap to Get-SchemaObjectName * Added $rootDse to Get-SchemaObjectName * Add inheritance options to NtfsAccessEntry (#34) It's easier to get these values from the Readme, than going through code each time. * Release 10/6/2021 * Support for capability SIDS Co-authored-by: Brett Slaski <brettski@yahoo.com> Co-authored-by: Jason Ryberg <jason@ryberg.dev> Co-authored-by: Matthew Collera <Matthew.Collera@microsoft.com> Co-authored-by: Reggie Gibson <31147354+regedit32@users.noreply.github.com> Co-authored-by: Brian Gouldman <32549363+bgouldman@users.noreply.github.com> Co-authored-by: Jason Walker <walkerjason@live.com> Co-authored-by: Brian Wilhite <bcwilhite@live.com> Co-authored-by: Jason Walker <jwalker@microsoft.com> Co-authored-by: Chase Wilson <31453523+chasewilson@users.noreply.github.com> Co-authored-by: Brett Slaski <brettski@yahoo.com> Co-authored-by: Jason Ryberg <jason@ryberg.dev> Co-authored-by: Matthew Collera <Matthew.Collera@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Code clean up and refactoring as well as fix for bug #46
Bug #46:
When Force and ForcePrincipal parameters are set to false and Inheritance/Propagation Flags are not equal to the currently defined ACE, the Test-TargetResource function will return $false for "InDesiredState". Because of this issue, the Set-TargetResource Function is run unnecessarily, as well as the configuration is never compliant/in desired state.
To Reproduce
On the target machine, modify an NTFS permission rule outside of the default setting and/or outside what the current configuration specifies. Furthermore, a principal can be defined via a configuration to have 1 set as the InheritanceFlags, but the actual ACE for the specific principal is 3, when this scenario occurs, the Test-TargetResource will return false and the Set runs. When the Set-TargetResource function runs, it sets the ACE to 1, however, since the principal already has 3, it has more than what is needed so nothing technically happens.
Create a Member Server configuration and apply it to a target machine.
When you run the configuration over and over again, note that it's never in the desired state, even though the Set-TargetResource function is executed.
Expected behavior
The Set-TargetResource should not run in this scenario since the specified rule is already applied via an ACE granting common Inheritance/Propagation Flags.