Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AccessRules - BugFix + Validation/warning on setting access rules #82

Open
wants to merge 5 commits into
base: development
Choose a base branch
from
Open

AccessRules - BugFix + Validation/warning on setting access rules #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
54cb1ff
Bugfix for "NoFixConfig" option within AccessRules
AndiBellstedt Jan 15, 2024
244436f
Add validation + Warning Message on Processing AccessRules when rule …
AndiBellstedt Jan 15, 2024
3b1531c
add changelog documentation
AndiBellstedt Jan 15, 2024
df150f1
add changelog documentation
AndiBellstedt Jan 15, 2024
b903357
Allow empty strings on user registration, due to UPN is allowed to be…
AndiBellstedt Mar 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions DomainManagement/changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Changelog

## ??? (???)

- Fix: Access Rules - "NoFixConfig" option within AccessRules wasn't respected. (issue #81)
- Upd: Access Rules - Add warning message when access rule is applied but redundant or simply not working in the acl object

## 1.8.202 (2024-01-12)

- Upd: Access Rules - added option to parallelize tests (experimental)
Expand Down
123 changes: 62 additions & 61 deletions DomainManagement/en-us/strings.psd1
View file
Open in desktop

Large diffs are not rendered by default.

62 changes: 42 additions & 20 deletions DomainManagement/functions/AccessRule/Invoke-DMAccessRule.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,32 @@
<#
.SYNOPSIS
Applies the desired state of accessrule configuration.

.DESCRIPTION
Applies the desired state of accessrule configuration.
Define the desired state with Register-DMAccessRule.
Test the desired state with Test-DMAccessRule.

.PARAMETER InputObject
Test results provided by the associated test command.
Only the provided changes will be executed, unless none were specified, in which ALL pending changes will be executed.

.PARAMETER Server
The server / domain to work with.

.PARAMETER Credential
The credentials to use for this operation.

.PARAMETER Confirm
If this switch is enabled, you will be prompted for confirmation before executing any operations that change state.

.PARAMETER WhatIf
If this switch is enabled, no actions are performed but informational messages will be displayed that explain what would happen if the command were to run.

.PARAMETER EnableException
This parameters disables user-friendly warnings and enables the throwing of exceptions.
This is less user friendly, but allows catching exceptions in calling scripts.

.EXAMPLE
PS C:\> Invoke-DMAccessRule -Server contoso.com

Expand All @@ -37,17 +37,17 @@
param (
[Parameter(ValueFromPipeline = $true)]
$InputObject,

[PSFComputer]
$Server,

[PSCredential]
$Credential,

[switch]
$EnableException
)

begin {
$parameters = $PSBoundParameters | ConvertTo-PSFHashtable -Include Server, Credential
$parameters['Debug'] = $false
Expand All @@ -62,13 +62,13 @@
if (-not $InputObject) {
$InputObject = Test-DMAccessRule @parameters
}

foreach ($testItem in $InputObject) {
# Catch invalid input - can only process test results
if ($testItem.PSObject.TypeNames -notcontains 'DomainManagement.AccessRule.TestResult') {
Stop-PSFFunction -String 'General.Invalid.Input' -StringValues 'Test-DMAccessRule', $testItem -Target $testItem -Continue -EnableException $EnableException
}

switch ($testItem.Type) {
'Update' {
Write-PSFMessage -Level Debug -String 'Invoke-DMAccessRule.Processing.Rules' -StringValues $testItem.Identity, $testItem.Changed.Count -Target $testItem
Expand All @@ -82,7 +82,7 @@
Write-PSFMessage -Level InternalComment -String 'Invoke-DMAccessRule.AccessRule.Remove' -StringValues $changeEntry.ADObject.IdentityReference, $changeEntry.ADObject.ActiveDirectoryRights, $changeEntry.ADObject.AccessControlType, $changeEntry.DistinguishedName -Target $changeEntry
$aclObject.RemoveAccessRuleSpecific($changeEntry.ADObject.OriginalRule)
Remove-RedundantAce -AccessControlList $aclObject -IdentityReference $changeEntry.ADObject.OriginalRule.IdentityReference

$stillThere = $false
foreach ($rule in $aclObject.GetAccessRules($true, $false, [System.Security.Principal.NTAccount])) {
if (Test-AccessRuleEquality -Parameters $parameters -Rule1 $rule -Rule2 $changeEntry.ADObject.OriginalRule) {
Expand All @@ -95,7 +95,7 @@
if ($stillThere -and $alternativeRemoval) {
$null = $aclObject.RemoveAccessRule($changeEntry.ADObject.OriginalRule)
Remove-RedundantAce -AccessControlList $aclObject -IdentityReference $changeEntry.ADObject.OriginalRule.IdentityReference

$stillThere = $false
foreach ($rule in $aclObject.GetAccessRules($true, $false, [System.Security.Principal.NTAccount])) {
if (Test-AccessRuleEquality -Parameters $parameters -Rule1 $rule -Rule2 $changeEntry.ADObject.OriginalRule) {
Expand All @@ -119,15 +119,26 @@
try {
if (-not $changeEntry.Configuration.ObjectType) { throw "Unknown ObjectType! Unable to translate $($changeEntry.Configuration.ObjectTypeName). Validate the configuration and ensure pending schema updates (e.g. Exchange, Skype, etc.) have been applied." }
if (-not $changeEntry.Configuration.InheritedObjectType) { throw "Unknown InheritedObjectType! Unable to translate $($changeEntry.Configuration.InheritedObjectTypeName). Validate the configuration and ensure pending schema updates (e.g. Exchange, Skype, etc.) have been applied." }
$accessRule = [System.DirectoryServices.ActiveDirectoryAccessRule]::new((Convert-Principal @parameters -Name $changeEntry.Configuration.IdentityReference), $changeEntry.Configuration.ActiveDirectoryRights, $changeEntry.Configuration.AccessControlType, $changeEntry.Configuration.ObjectType, $changeEntry.Configuration.InheritanceType, $changeEntry.Configuration.InheritedObjectType)
$accessRule = [System.DirectoryServices.ActiveDirectoryAccessRule]::new(
(Convert-Principal @parameters -Name $changeEntry.Configuration.IdentityReference),
$changeEntry.Configuration.ActiveDirectoryRights,
$changeEntry.Configuration.AccessControlType,
$changeEntry.Configuration.ObjectType,
$changeEntry.Configuration.InheritanceType,
$changeEntry.Configuration.InheritedObjectType
)
}
catch {
$failedCount = $failedCount + 1
Stop-PSFFunction -String 'Invoke-DMAccessRule.AccessRule.Creation.Failed' -StringValues $testItem.Identity, $changeEntry.Configuration.IdentityReference -EnableException $EnableException -Target $changeEntry -Continue -ErrorRecord $_
}
$null = $aclObject.AddAccessRule($accessRule)
#TODO: Validation and remediation of success. Adding can succeed but not do anything, when accessrules are redundant. Potentially flag it for full replacement?
continue
# Validation and remediation of success. Adding can succeed but not do anything, when accessrules are redundant. Potentially flag it for full replacement?
if (-not ($aclObject.Access | Where-Object { $_ -in $accessRule })) {
$failedCount = $failedCount + 1
Write-PSFMessage -Level Warning -String 'Invoke-DMAccessRule.AccessRule.Creation.NotApplied' -StringValues $testItem.Identity, $changeEntry.Configuration.IdentityReference -Target $changeEntry
}
continue
}
#endregion Add Access Rules

Expand All @@ -137,14 +148,25 @@
try {
if (-not $changeEntry.Configuration.ObjectType) { throw "Unknown ObjectType! Unable to translate $($changeEntry.Configuration.ObjectTypeName). Validate the configuration and ensure pending schema updates (e.g. Exchange, Skype, etc.) have been applied." }
if (-not $changeEntry.Configuration.InheritedObjectType) { throw "Unknown InheritedObjectType! Unable to translate $($changeEntry.Configuration.InheritedObjectTypeName). Validate the configuration and ensure pending schema updates (e.g. Exchange, Skype, etc.) have been applied." }
$accessRule = [System.DirectoryServices.ActiveDirectoryAccessRule]::new((Convert-Principal @parameters -Name $changeEntry.Configuration.IdentityReference), $changeEntry.Configuration.ActiveDirectoryRights, $changeEntry.Configuration.AccessControlType, $changeEntry.Configuration.ObjectType, $changeEntry.Configuration.InheritanceType, $changeEntry.Configuration.InheritedObjectType)
$accessRule = [System.DirectoryServices.ActiveDirectoryAccessRule]::new(
(Convert-Principal @parameters -Name $changeEntry.Configuration.IdentityReference),
$changeEntry.Configuration.ActiveDirectoryRights,
$changeEntry.Configuration.AccessControlType,
$changeEntry.Configuration.ObjectType,
$changeEntry.Configuration.InheritanceType,
$changeEntry.Configuration.InheritedObjectType
)
}
catch {
$failedCount = $failedCount + 1
Stop-PSFFunction -String 'Invoke-DMAccessRule.AccessRule.Creation.Failed' -StringValues $testItem.Identity, $changeEntry.Configuration.IdentityReference -EnableException $EnableException -Target $changeEntry -Continue -ErrorRecord $_
}
$null = $aclObject.AddAccessRule($accessRule)
#TODO: Validation and remediation of success. Adding can succeed but not do anything, when accessrules are redundant. Potentially flag it for full replacement?
$null = $aclObject.AddAccessRule($accessRule)
# Validation and remediation of success. Adding can succeed but not do anything, when accessrules are redundant. Potentially flag it for full replacement?
if(-not ($aclObject.Access | Where-Object { $_ -in $accessRule })) {
$failedCount = $failedCount + 1
Write-PSFMessage -Level Warning -String 'Invoke-DMAccessRule.AccessRule.Creation.NotApplied' -StringValues $testItem.Identity, $changeEntry.Configuration.IdentityReference -Target $changeEntry
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a particular problem you added those for or was that more of a "Oh. there's a TODO marker there, lets add that while I'm on it"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I was digging around with the AccessRules and did some testing. As far as I remember -and it was a while ago, so my mind is a bit blurry on the exact situation- I did consider that as a bug.
ADMF did not throw information on NoFix-Items and on edge-case-scenarios.
Something like a ACE is not present on the object but defined on schema default ACL. Something like this, but as said, I did not remember exactly.
...and in the end... in fact it was marked as ToDo and did make sense to me.

continue
}
#endregion Restore Default Access Rules
Expand Down
39 changes: 20 additions & 19 deletions DomainManagement/functions/AccessRule/Test-DMAccessRule.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<#
.SYNOPSIS
Validates the targeted domain's Access Rule configuration.

.DESCRIPTION
Validates the targeted domain's Access Rule configuration.
This is done by comparing each relevant object's non-inherited permissions with the Schema-given default permissions for its object type.
Expand All @@ -19,13 +19,13 @@
- Have at least one path based rule.
- Are considered as "under management", as defined using Set-DMContentMode
It uses a definitive approach - any access rule not defined will be flagged for deletion!

.PARAMETER Server
The server / domain to work with.

.PARAMETER Credential
The credentials to use for this operation.

.EXAMPLE
PS C:\> Test-DMAccessRule -Server fabrikam.com

Expand All @@ -39,11 +39,11 @@
param (
[PSFComputer]
$Server,

[PSCredential]
$Credential
)

begin {
#region Utility Functions
function Convert-AccessRule {
Expand Down Expand Up @@ -95,6 +95,7 @@
ObjectTypeName = $objectTypeName
PropagationFlags = $ruleObject.PropagationFlags
Present = $ruleObject.Present
NoFixConfig = $ruleObject.NoFixConfig
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching that one

}
}
}
Expand Down Expand Up @@ -150,7 +151,7 @@
}

$adObject = Get-ADObject @parameters -Identity $resolvedPath -Properties adminCount

if ($adObject.adminCount) {
$defaultPermissions = @()
$desiredPermissions = Get-AdminSDHolderRules @parameters
Expand All @@ -173,53 +174,53 @@
#region Process Non-Configured AD Objects - Serial
if (-not $doParallelize) {
$resolvedConfiguredObjects = $script:accessRules.Keys | Resolve-String

$foundADObjects = foreach ($searchBase in (Resolve-ContentSearchBase @parameters -NoContainer)) {
Get-ADObject @parameters -LDAPFilter '(objectCategory=*)' -SearchBase $searchBase.SearchBase -SearchScope $searchBase.SearchScope -Properties adminCount
}

$resultDefaults = @{
Server = $Server
ObjectType = 'AccessRule'
}

$convertCmdName = { Convert-DMSchemaGuid @parameters -OutType Name }.GetSteppablePipeline()
$convertCmdName.Begin($true)
$convertCmdGuid = { Convert-DMSchemaGuid @parameters -OutType Guid }.GetSteppablePipeline()
$convertCmdGuid.Begin($true)

$processed = @{ }
foreach ($foundADObject in $foundADObjects) {
# Prevent duplicate processing
if ($processed[$foundADObject.DistinguishedName]) { continue }
$processed[$foundADObject.DistinguishedName] = $true

# Skip items that were defined in configuration, they were already processed
if ($foundADObject.DistinguishedName -in $resolvedConfiguredObjects) { continue }

$adAclObject = Get-AdsAcl @parameters -Path $foundADObject.DistinguishedName
$compareParam = @{
ADRules = $adAclObject.Access | Convert-AccessRuleIdentity @parameters
DefaultRules = Get-DMObjectDefaultPermission @parameters -ObjectClass $foundADObject.ObjectClass
ConfiguredRules = Get-CategoryBasedRules -ADObject $foundADObject @parameters -ConvertNameCommand $convertCmdName -ConvertGuidCommand $convertCmdGuid
ADObject = $foundADObject
}

# Protected Objects
if ($foundADObject.AdminCount) {
$compareParam.DefaultRules = @()
$compareParam.ConfiguredRules = Get-AdminSDHolderRules @parameters
}

$compareParam += $parameters
$delta = Compare-AccessRules @compareParam

if ($delta) {
New-TestResult @resultDefaults -Type Update -Changed $delta -ADObject $adAclObject -Identity $foundADObject.DistinguishedName
continue
}
}

$convertCmdName.End()
$convertCmdGuid.End()

Expand Down Expand Up @@ -345,7 +346,7 @@
$null = $workflow | Add-PSFRunspaceWorker @param

$resolvedConfiguredObjects = $script:accessRules.Keys | Resolve-String

$foundADObjects = foreach ($searchBase in (Resolve-ContentSearchBase @parameters -NoContainer)) {
Get-ADObject @parameters -LDAPFilter '(objectCategory=*)' -SearchBase $searchBase.SearchBase -SearchScope $searchBase.SearchScope -Properties adminCount
}
Expand All @@ -370,7 +371,7 @@
foreach ($fail in $fails) {
Write-PSFMessage -Level Warning -String 'Test-DMAccessRule.Parallel.Error' -StringValues $fail.ADObject -ErrorRecord $fail.Error -Target $fail
}

$results = Read-PSFRunspaceQueue -InputObject $workflow -Name results -All
# Fix String Presentation for objects from a background runspace
$results | Add-Member -MemberType ScriptMethod -Name ToString -Value { $this.Identity } -Force
Expand Down
Loading