CircularIndirect for non-circular groups

[Mike-Crowley]

If you create 4 groups with membership like this:

"A","B","C","D" | foreach {New-ADGroup $_ -GroupScope Universal}
Add-ADGroupMember A -Members "B"
Add-ADGroupMember B -Members "C","D"
Add-ADGroupMember C -Members "D"

The following command:

Get-WinADGroupMemberOf D -ClearCache | select ObjectName, ParentGroup, Name, CircularIndirect

reports the following:

ObjectName ParentGroup Name CircularIndirect
---------- ----------- ---- ----------------
D          D           C               False
D          C           B               False
D          B           A               False
D          D           B                True
D          B           A               False

In some circumstances, this is a poor AD group design, but it is not "circular", right? Or am I misunderstanding the intent of this field?

[PrzemyslawKlys]

In this context or any context Circular's job is to prevent going in circles scanning groups over and over again, essentially never ending. It doesn't do super duper verification - it's there to prevent circles.

The script scans group A - then goes into group B - then it has to go thru C and D as part of B. Once it goes into C it expands further, it goes into D, and goes back to group B to finish rest of the members - it now sees that group D is in group B but it was already scanned so it marks it as circular indirect and stops. This is to prevent possibility that D leads again to B or A and goes into circles.

While in this case it will not go into circles, it's marking group to stop going again in to group D.

[Mike-Crowley]

Thanks for your quick response. I can appreciate the need for something to prevent an infinite loop in the code, but since it is reported in the output, especially alongside the circularDirect attribute, I was expecting it to alert me to situations where the membership itself was circular. e.g. A is a member of B and B is a member of A.

Perhaps we could add some additional logic to do both?

[PrzemyslawKlys]

Feel free to dive into the logic of it and try to fix it. It's easier said than done. I guess you would need to track the value, but once it's out of the diving into the first one, reset it and go again, and then somehow detect real Circulars with more precision.

[Mike-Crowley]

I totally agree. You've put a lot of thought into this module (which is amazing BTW). If I can think of a good way to contribute, I'll circle back.

[Mike-Crowley]

I've got the detection working as a standalone feature, though I need to add some polish and resiliency.

As for getting it integrated, is the logic in Get-WinADGroupMemberOf.ps1 only, or distributed throughout the repo? I haven't read through the entire code base.

I'm also thinking another column would be useful. In addition to improving the accuracy of CircularIndirect, a field called CircularPath could have values for the circle, like this:

CircularIndirect	CircularPath
True	A -> B -> C -> D -> A
False
True	B -> C -> D -> A -> B

Thoughts?

Feel free to tell me to go away as well, if you'd rather not spend the cycles discussing this.

[PrzemyslawKlys]

There are 2 functions Get-WinADGroupMember, Get-WinADGroupMemberOf that deal with circular. Similar but not identical logic. You can submit PR and we can take it from there. I've in my domain couple of bad test groups that normally would blow up without this logic. You can add CircularPath, but I guess you will be adding there DN? Name? DisplayName? That path looks great on your ABCDE, but I wonder what will happen with that when it's 50+ chars group names.

I like how it looks - just seeing some issues with it. Also take a a look on Circular field itself. I don't remember what's the difference between those 2.

[PrzemyslawKlys]

SO how is that going for you? :-) It's been 6+ months -> did you ever manage to improve it?

[Mike-Crowley]

Sorry for dropping the ball here. Life had other plans for my good intentions. If you mean remind me, I am still interested on doing this work, but if you are looking to close the issue, I'd understand that too.

[PrzemyslawKlys]

It can stay open :-) I have my list of things to do, just was curious :D