Find related groups among list of users

Question

I have one of my first PowerShell scripts that will query a list of users in Active Directory. While doing so it will keep a tally in a hashtable that will update for every group increasing it count so that a final total can then be displayed.

This is used so that given a group of users that are performing the same job I can create a role based security group in AD and move all the related groups into it. Thereby reducing the amount of work and thought involved when creating new users and making changes to existing departments.

<#
    Script     : Count Group Memberships
    Created By : Matt Something
    Created    : November 28, 2012
    Modified   : November 28, 2012

    This script will take a list of samaccountnames from active directory and count the membership lists between them. This
    is used to track the similarities between accounts.  
#>

param (
    [parameter(ValueFromRemainingArguments=$true)]
    [string[]]$userNamesToCheck = $(throw "Need to supply at least one username")
)

# Functions and Subroutines
Function Get-NamedModule { 
    Param([string]$name) 
    if(-not(Get-Module -name $name)) { 
        if(Get-Module -ListAvailable | Where-Object { $_.name -eq $name }) { 
            # Check the list of available module for $name. If present import it.
            Import-Module -Name $name 
            $true 
        } 
        else { $false } # Module not available 
    } # End if
    else { $true } # Module already loaded 
} # End function 

# Variable Declaration
$counts = @{}

# Main
# Import the activedirectory module if its not already. 
[void](Get-NamedModule -name "activedirectory")

# Cycle through usernamestocheck and get their group memberships
$userNamesToCheck | ForEach-Object{
    # Get the group memberships from each user.
    $memberships = (Get-AdUser -Filter { SamAccountName -eq $_ } -Properties MemberOf).MemberOf

    # Add the groups to a table for counting
    $memberships | ForEach-Object{
        [void]($_ -match "^CN=(?<membership>.*?),")

        # Continue to build the table
        $singleGroup = $matches.membership

        If($counts.ContainsKey($singleGroup)){
            # This group already exists in the table.
            $counts.Set_Item($singleGroup, [int]($counts.($singleGroup)) + 1)
        } else {
            $counts.Add($singleGroup, 1)
        }
    }
}

Write-Host "$((Measure-Object -InputObject $userNamesToCheck).Count) users checked"
$userNamesToCheck
$counts.GetEnumerator() | Sort-Object -Property Value -Descending

Answer 1

Purposeful PowerShell Help

I see that you have some comments in your script. Those for the most part would be hidden inside the script and the only way someone would see them is to open it. This may or not be what you intended but PowerShell has its own help based comment system that can contain this information as well as make it easy for running users to be informed.

About the parameters

You are using throw with the idea that you want the parameter to be mandatory with a message to show. That functionality is better handled by advanced function parameters

param (
    [parameter(Mandatory=$true,
        HelpMessage='Expecting an array of samaccountnames')]
    [string[]]$userNames
)

Mandatory is self explanatory and HelpMessage

Windows PowerShell displays this message in the prompt that appears when a mandatory parameter value is missing from a command.

Module checker

Obviously this script is dependent on the activedirectory module being present. You have created a function that will load the module if it has not been already. The is some improvement that can be made of this function. Namely no sense in checking twice for the module presence. It is also good practice to use approve verbs properly. Get could be misleading in this case when in fact you are trying to Confirm

After looking at it a little more this can be reduced to one line that can be evaluated as a boolean which should accomplish the same thing. If you choose to keep the function keep in mind the points I made above. We only need it once so it does not really need to be in a function. Perhaps it should be in a local PowerShell profile.

if(!(Import-Module -Name $name -PassThru -ErrorAction SilentlyContinue)){
    # Script should exit will useful message as we cannot continue.
}

There are 3 possible scenarios for when this is run.

• Module $name is not loaded but available: Command will execute and import the module $name. Once imported the module object will be returned because of -PassThru. The command is cast as [boolean] would return true.
• Module $name is loaded: The module object will be returned because of -PassThru. The command is cast as [boolean] would return true.
• Module $name is not available: The command will silently fail and $null should be returned. The command is cast as [boolean] would return false.

Grouping Logic

The way the hashtable is built here obviously works but can be completely replaced with the use of the cmdlet Group-Object. I would suspect that, at the time, you were not aware of it. It would handle the grouping and the counts that we need for this. Also there is no test to see if the users actually exist. Need to account for that else the script will just hard fail.

You can see this in the completed solution below.

Updated solution

All the points collected above are used down here.

<#
.SYNOPSIS 
Get and groups AD groups of specified users

.DESCRIPTION
This script will take a list of samaccountnames from 
active directory and count the membership lists between them. 
This can be used to track the group similarities between accounts. 

.PARAMETER Users
Specifies the array of samaccount names to be looked up. 

.EXAMPLE
C:\PS> extension -Users jsmith, bjangles
#>

param (
    [parameter(Mandatory=$true,
        HelpMessage='Expecting an array of samaccountnames')]
    [string[]]$Users
)

if(!(Import-Module -Name activedirectory -PassThru -ErrorAction SilentlyContinue)){
    # Script should exit will useful message as we cannot continue.
    throw("The module $name must be present in order to continue.")
}

$userMemberships = ForEach($user in $users){
    try{
        # Get the memberships of this user. 
        #Since it is possible the user has no groups we need to separate the logic for that and failure
        (Get-Aduser -Identity $user -Properties memberOf -ErrorAction SilentlyContinue).memberof | ForEach-Object{
            [pscustomobject]@{
                User = $User
                Group = $_
            }
        } 
    } catch {
        Write-Warning "Error retriving the group membership of $user"
    }
}

# Collect all the groups and display counts
$userMemberships | 
    Group-Object -Property Group | 
    Select @{Name="Group";Expression={(Get-ADGroup $_.Name).Name}},Count | 
    Sort-Object Count -Descending

# Output the number of users check so the output has context
Write-Host "$($users.Count) users checked" -ForegroundColor Green

Minor comments

Principal Group Members

For me this is just the Domain Users group. Potentially in other orgs they use the principal group member for something else.

Accounting for bad users

You might consider adding more verbose output concerning errors. If there is a user that could not be queried then the last line could be considered faulty. I would think the warning would be indication enough.

Calculated Expression

You will see a calculated expression used in the final output. The groups returned from Get-ADUser contain the full DistinguisedName of the groups. Using a calculated expression we just get the name from the dn. While you could use string manipulation the cmdlet is more efficient and precise.