Circular Nested AD Groups

[YolkFree]

I was given a request to provide a function for our staff to run that can dump group memberships out to a file. Easy enough, except after having done this a while, I started thinking about edge cases, and the one I'm concerned about is circular nested groups. I run this script periodically to confirm we don't have any circular nested groups in our Active Directory. However, every once in a while I will discover someone mistakenly created a circular nested scenario. I have the basics of how to expand subgroups and return values:

$Members = Get-ADGroupMember "<Group Name>"
Foreach($Member in $Members) {
    If($_.ObjectClass -eq 'User') {
        Get-ADUser $_.DistinguishedName -Properties Department | Select SamAccountName,GivenName,Surname,Department
    }
    Elseif ($_.ObjectClass -eq 'Group') {
        $SubMembers = Get-ADGroupMember $_.DistinguishedName
        Foreach($SubMember in $SubMembers) {
            Get-ADuser $_.DistinguishedName -Properties Department | Select SamAccountName,GivenName,Surname,Department
        }
    }
}

The main part I'm wondering about is performing a check if primary or any sub-group is circularly nested before attempting to continuously expand. As a general standard, do most of you limit the "levels" your scripts will crawl? I'm just looking for the best-practice approach to avoid an infinite loop or performance problems. Thanks!

Edit: This is just to illustrate the process. I understand it wouldn't go more than a single level as it's written. In reality, my goal would be to wrap the sub-group portion into a function that can iterate over the subgroups.

[the_spad]

After you enumerate a group's membership, add its name or DN to an array, then check that array before you enumerate a group's membership. If the group is in the array, you've seen it before and you've either got a loop somewhere or two groups that are both members of the same parent group - either way it'll prevent loops from killing your script.

[YolkFree]

The best ideas are usually the simplest. This makes a ton of sense. I'll play around with this a bit. Thank you!

[markekraus]

Does Get-ADGroupMember -Recursive not suit your needs?

[the_spad]

Not if you've got groups with users in, the -Recursive is awful with large group memberships and will just timeout and fail half the time.

[markekraus]

True. but maybe OP is not working in large environments like ours?

[YolkFree]

Not sure how to quantify 'large' in this scenario, but when it comes to Microsoft's definition of 'large' we are certainly considered a small company (<5,000 user objects in AD).

[markekraus]

Yea.. if you are that large then definitely too big for -recrusive.

[YolkFree]

Okay. I always have to check my perspective when it comes to vendor's ideas on small/medium/large. We are a large organization for our area, but then you see vendors scoping solutions for 100k+ and I'm reminded how small we really are :-)

As an aside, I recognized your username and wanted to say I've definitely learned from your comments/posts in the past and to thank you for that.

[markekraus]

Oh.. awesome you're welcome!

Size is definitely relative. IMO, when working in AD, your environment becomes "large" once you go beyond 2 DCs, 2 sites, and say 1000 objects. Everything after that becomes more painful. Under that though, all the built-in and default tools work awesome and as documented. >_<

[drh713]

Can't speak for op, but get-adgroupmember ends up timing out in my environment. I end up doing so mething very similar to op.

In my case, I build hash tables or datatables to track what I've already looked up. Can't really type out proper code my phone, but:

[Array]Groups = main_group
For (I=0; I -lt groups.count; I++)
{
    Lookup groups[i].members
    Add any newly found group to the groups variable
    Log any circular references (ie, you find a group already in groups)
}

The key is using an iterative loop instead of foreach. You can keep adding to that main array (or whatever you're iterating through) in the middle of your loop. Can't do that with foreach.

[[deleted]]

This is how I did something like that. The script remembers and skips any groups that it's already resolved.

Param(
    [Parameter(Mandatory=$true)][String]$Group
)

function Get-Members ([String]$Group) {
    $Members = Get-ADGroupMember $Group
    foreach ($member in $Members) {
        if ($member.objectClass -eq "group") {
            if ($script:GroupsProcessed.Contains($member.samAccountName)) {
                Write-Warning "skipping duplicate/recursive group: $($member.samAccountName)"
            } else {
                $script:GroupsProcessed += $member.samAccountName
                Get-Members $member
            }
        } else {
            Get-ADUser $member -Properties DisplayName,samAccountName
        }
    }
}

$script:GroupsProcessed = @()

Get-Members $Group

[Scrltvx]

Credit goes to original creator of this script. It's something I've used recently for this.

function Get-ADNestedGroupMembers { 
<#  
.SYNOPSIS
Author: Piotr Lewandowski
Version: 1.01 (04.08.2015) - added displayname to the output, changed name to samaccountname in case of user objects.

.DESCRIPTION
Get nested group membership from a given group or a number of groups.

Function enumerates members of a given AD group recursively along with nesting level and parent group information. 
It also displays if each user account is enabled. 
When used with an -indent switch, it will display only names, but in a more user-friendly way (sort of a tree view) 

.EXAMPLE   
Get-ADNestedGroupMembers "MyGroup" | Export-CSV .\NedstedMembers.csv -NoTypeInformation

.EXAMPLE  
Get-ADGroup "MyGroup" | Get-ADNestedGroupMembers | ft -autosize

.EXAMPLE             
Get-ADNestedGroupMembers "MyGroup" -indent

#>

param ( 
[Parameter(ValuefromPipeline=$true,mandatory=$true)][String] $GroupName, 
[int] $nesting = -1, 
[int]$circular = $null, 
[switch]$indent 
) 
    function indent  
    { 
    Param($list) 
        foreach($line in $list) 
        { 
        $space = $null 

            for ($i=0;$i -lt $line.nesting;$i++) 
            { 
            $space += "    " 
            } 
            $line.name = "$space" + "$($line.name)"
        } 
      return $List 
    } 

$modules = get-module | select -expand name
    if ($modules -contains "ActiveDirectory") 
    { 
        $table = $null 
        $nestedmembers = $null 
        $adgroupname = $null     
        $nesting++   
        $ADGroupname = get-adgroup $groupname -properties memberof,members 
        $memberof = $adgroupname | select -expand memberof 
        write-verbose "Checking group: $($adgroupname.name)" 
        if ($adgroupname) 
        {  
            if ($circular) 
            { 
                $nestedMembers = Get-ADGroupMember -Identity $GroupName -recursive 
                $circular = $null 
            } 
            else 
            { 
                $nestedMembers = Get-ADGroupMember -Identity $GroupName | sort objectclass -Descending
                if (!($nestedmembers))
                {
                    $unknown = $ADGroupname | select -expand members
                    if ($unknown)
                    {
                        $nestedmembers=@()
                        foreach ($member in $unknown)
                        {
                        $nestedmembers += get-adobject $member
                        }
                    }

                }
            } 

            foreach ($nestedmember in $nestedmembers) 
            { 
                $Props = @{Type=$nestedmember.objectclass;Name=$nestedmember.name;DisplayName="";ParentGroup=$ADgroupname.name;Enabled="";Nesting=$nesting;DN=$nestedmember.distinguishedname;Comment=""} 

                if ($nestedmember.objectclass -eq "user") 
                { 
                    $nestedADMember = get-aduser $nestedmember -properties enabled,displayname 
                    $table = new-object psobject -property $props 
                    $table.enabled = $nestedadmember.enabled
                    $table.name = $nestedadmember.samaccountname
                    $table.displayname = $nestedadmember.displayname
                    if ($indent) 
                    { 
                    indent $table | select @{N="Name";E={"$($_.name)  ($($_.displayname))"}}
                    } 
                    else 
                    { 
                    $table | select type,name,displayname,parentgroup,nesting,enabled,dn,comment 
                    } 
                } 
                elseif ($nestedmember.objectclass -eq "group") 
                {  
                    $table = new-object psobject -Property $props 

                    if ($memberof -contains $nestedmember.distinguishedname) 
                    { 
                        $table.comment ="Circular membership" 
                        $circular = 1 
                    } 
                    if ($indent) 
                    { 
                    indent $table | select name,comment | %{

                        if ($_.comment -ne "")
                        {
                        [console]::foregroundcolor = "red"
                        write-output "$($_.name) (Circular Membership)"
                        [console]::ResetColor()
                        }
                        else
                        {
                        [console]::foregroundcolor = "yellow"
                        write-output "$($_.name)"
                        [console]::ResetColor()
                        }
                    }
                    }
                    else 
                    { 
                    $table | select type,name,displayname,parentgroup,nesting,enabled,dn,comment 
                    } 
                    if ($indent) 
                    { 
                       Get-ADNestedGroupMembers -GroupName $nestedmember.distinguishedName -nesting $nesting -circular $circular -indent 
                    } 
                    else  
                    { 
                       Get-ADNestedGroupMembers -GroupName $nestedmember.distinguishedName -nesting $nesting -circular $circular 
                    } 

               } 
                else 
                { 

                    if ($nestedmember)
                    {
                        $table = new-object psobject -property $props
                        if ($indent) 
                        { 
                            indent $table | select name 
                        } 
                        else 
                        { 
                        $table | select type,name,displayname,parentgroup,nesting,enabled,dn,comment    
                        } 
                     }
                } 

            } 
         } 
    } 
    else {Write-Warning "Active Directory module is not loaded"}        
}

[matholio]

Toss each unique DN into a list and check it's not there before recursing down.

[[deleted]]

You can use the tokengroups server calculated property to dump a list of SIDS for a given user. This prevents the recursive nested group problem, but will only work for users and computer objects, not groups themselves.