Grant-ADDTDGRights.ps1 set on each OU individually, which slows down script.

[PatrickOnGit]

The function Grant-ADDTDGRights.ps1 is writing and committing each ACL separately, which slows down the tool significantly. It should take all permissions for an OU, check which permission is already set, and add or remove the "managed" permissions in a single commit operation. Specifically, if the script is used multiple times, it must not try to set already existing ACLs.

[merddyin]

Actually this was already fixed in the uncommitted code base. Because this is a security oriented operation, it does perform a check, but if anything is missing or incorrect, it actually clears any static ACLs and rewrites all of them. It still performs a loop for each of the ACLs at present, but it only runs the commit action once for a given set of ACLs related to a particular group. If you have some ideas on ways to make things more efficient, I'm certainly open to suggestions or contributions.

[PatrickOnGit]

The current published code does not do much checks. Unfortunately, this part of the code is for a bit of a black box. It does too much looping through variables and finding items using where clause, which of course doesn't stop at the first item found, which makes it even more challanging to follow. So I had to stop improving as I didn't want to break it.

So all I did you can find in attached file:

Grant-ADDTDGRights.ps1

To compare ACEs from file system, I usually use this function:

Test-AXACL.ps1

It can be changed to support AD ACLs

Similar to this :

$Compare = Compare-Object -ReferenceObject $ReferemceDelegation -DifferenceObject $CurrentDelegation -Property "AccessControlType", 
                    "ActiveDirectoryRights", "IdentityReference", "InheritedObjectType",
                    "IsInherited", "ObjectFlags", "ObjectType", "PropagationFlags"

The challange is to ensure that $CurrentDelegation contains only those permissions which are "managed" and really need to be compared. So exclude all inherited permission or other way around, include only explicit permissions. But either exclude schema default permissions are add them to the $ReferenceDelegation.

As said, I would have done it, if I would have understand your code building the desired ACLs. Ideally you would read them directly from SQLite and avoid any use of the where-object. This makes the code slow and difficult to debug.

[merddyin]

Ok, that is a fair point on detangling the code. Perhaps I can provide a bit of insight in that regard.

In essence, the function expects to receive an OU path in some form, and that path needs to represent a Class OU for the Admin or Standard Focus paths, an Org (SLO) OU for Server, or the Provision/Deprovision for Staging. The first chunk of the code is about validating an devolving the path to figure out what needs to be processed. Once it knows information such as the focus, the tier, and the class, it queries the data sets to retrieve just the property group combinations required to be processed. Once it has this, it retrieves all of the mappings and determines the actual TDG name. Next, it retrieves all of the property mappings and creates a collection of ACLs that need to be assigned, based on the rights and scoping data defined for each property group. The next step is to take the collected data and turn it into actual ActiveDirectoryAccessRule values. As you rightly pointed out, the current version of the code in the repo then simply cycles through each defined access rule, adds it, and commits it.

When I initially wrote this version of this function, I initially simply added all of the rules and then called the commit only a single time. During testing however, I would sometimes receive a failure on the commit stage that would inevitably be traced back to a single rule that was blocking the commit. This always took a considerable amount of trial and error to trace. Switching to use individual rule commits eliminated this added work and also improved the results, since all the other rules would correctly write to the object. Writing the rule every time the tools is run however is, as you pointed out, inefficient.

As for fixing it, there may be an easier method than to call a separate function. The System.DirectoryServices.DirectoryEntry.ObjectSecurity class contains a list of ActiveDirectorySecurity class objects applied to the AD object. Since this class supports IEquatable, it has an 'Equals' method, so it should support -contains. What I could theoretically do is create a collection of non-inherited rules, then loop through them to see if my rules collection to be written contains the rule from the object. If it does, meaning I have a match, I remove that item from the list to add. If the rule isn't found, then I remove it from the object. Finally, I can add all of the rules to be written that weren't filtered out, and then run a single commit like I did in the past. I honestly haven't encountered a commit issue for ACLs in some time, as I worked out all the bugs with that process.

As for all the Where clauses, this is admittedly intentional. While it would theoretically be possible to just create a bunch of collections for every possible scenario, the fact is that this would add a lot of additional code, and could make the module less dynamic. As things stand now, if you enable a class from the AP_Objects table, or even if you add a new one, not a single line of code has to be changed...as long as the related bits are in the DB, everything still works because I am using those Where clauses to filter the values. The issue around traceability, in this instance, is because it isn't clear what is in those various variables that I create on load without a lot of added effort. As mentioned before, I have a helper function used to support discoverability for other developers. I have created a new branch, called 'Dev', and I have added this helper function there. I'll do my best to wrangle some time to get all of the updated code loaded up in the next week or so to the same branch.