Okay, so I went through and had a thorough look at your script (late at night because it was fun, sue me, I’m weird with code alright). I changed quite a bit, removed a lot of redundant or unused variables, and pulled the double foreach loops into a single one to consolidate it a bit better.
I reformatted a lot of your long pipelines to use nice neat line breaks (you can break lines after a pipe character to continue the pipeline on the next line, if you weren’t aware!) and keep things more readable.
I renamed a couple variables for readability and streamlined your queries – there didn’t appear to be a real need to query AD twice here, since you were pulling the same information.
Also splatted the email command since you were halfway there anyway… and added some begin{} process{} end{} blocks for syntactic sugar. May also make it easier to put direct pipeline input logic into if you ever decide to go that route as well!
Finally, I think I actually stumbled across your actual issue. You were trying to add a new property containing the group name to the objects you’re getting back from AD for the master CSV file, as I read it.
In order to do that, there are a few ways, but by far one of the quickest and most clear syntactically is with Select-Object:
$ADUsers | Select-Object *, @{
Name = "Group"
Expression = {$Group}
}
Basically, you construct a simple hashtable containing the name of the property you’re adding and its value. The extra braces are, to my memory, necessary. The * is there to say “I want all the properties it already has” and then we add an extra one. This is known as a ‘calculated property’ if you wanna do some more reading.
And if you’re curious to how I managed to find that in that script, well… it doesn’t look the same to me anymore
[CmdletBinding()]
param(
[Parameter(Position = 0, Mandatory, HelpMessage = "Pass the pattern of the group you wish to look for")]
[ValidateNotNullOrEmpty()]
[string]
$Client,
[Parameter()]
[ValidateNotNullOrEmpty()]
[string]
$Domain = (Get-WmiObject Win32_ComputerSystem).Domain
)
begin {
$WorkingDirectory = Join-Path -Path $PSScriptRoot -ChildPath $Client
$MasterCsvPath = Join-Path -Path $WorkingDirectory -ChildPath 'Master.csv'
$ClientZipPath = Join-Path -Path $WorkingDirectory -ChildPath "$Client.zip"
# Create directory structure if needed
if (-not (Test-Path -Path $WorkingDirectory )) {
New-Item -ItemType Directory -Path $WorkingDirectory
}
}
process {
# Get the group list based on client name and domain
$GroupList = Get-ADGroup -Filter "SamAccountName -like '*$Client*'" -Server $Domain |
Select-Object -ExpandProperty SamAccountName
# Create list by group
foreach ($Group in $GroupList) {
$GroupCsvPath = Join-Path -Path $WorkingDirectory -ChildPath "$Group.csv"
# Compile group data that is needed for both CSV exports
$GroupData = Get-ADGroupMember -Identity $Group -Server $Domain |
Where-Object ObjectClass -eq 'User' |
Get-ADUSer -Server $Domain -Properties SamAccountName, GivenName, Surname |
Select-Object -Property SamAccountName, GivenName, Surname
# Export the per-group CSV
$GroupData |
Export-Csv -Path $GroupCsvPath -NoTypeInformation
# Export the master CSV after tacking on a property that references the current group
$GroupData | Select-Object *, @{
Name = "Group"
Expression = {$Group}
} | Export-Csv -Path $MasterCsvPath -NoTypeInformation
}
}
end {
Compress-Archive -Path "$WorkingDirectory\*.csv" -DestinationPath $ClientZipPath
$MailParams = @{
To = @("deleted")
From = "NoReply@"
Subject = "User-Group Listing for $Client"
Attachments = $ClientZipPath
SmtpServer = "appmailrelay"
}
Send-MailMessage @MailParams
}
Couple final notes:
- You can actually just "drop a $variable in a string" like that. As long as it's a double quoted string, PS will take care of the ins and outs of how to get the value in there for you. Single quoted strings are strictly literal, though.
- Although I've severely overused it here, I would recommend utilising Join-Path to join two path portions together. While you can mostly keep track of it in your own code, as soon as you start taking user input that might have, for example, backslashes in it, you need to think about using Join-Path to save you the hassle of properly splicing the paths together.
- Speaking of paths... I note you're using a lot of implied relative paths. In general, avoid this. It's very unclear what you're trying to do. If you must use a relative path, I'd advise being clear about it using the '.\Thing\File.txt' form -- that first dot specifies the current working directory, and it'll save you from strange edge cases. That said, most times when you're talking current directory you actually mean the directory the script is in -- these can be different, you can call a script using its full path from anywhere. As a result, you should use $PSScriptRoot in order to properly pull the script's directory and work from there.
- 7zip works really well, but I would caution against using it unless you really need to. You should generally seek to make your scripts as versatile as possible, and that means minimising external dependencies. If you need 7zip (for example, the native Compress-Archive cmdlets don't support files over 2GB in size) for an edge case or specific purpose, by all means, use it. But where it's not really necessary, try to stick to native PS methods.
- Finally, rather than trying to work directly with your parameters, I would advise simply letting PowerShell's built in frameworks handle it for you. You can apply a variety of attributes to your parameters in order to define exactly what input you'll accept, and PS will enforce it for you. If you fail to enter a mandatory parameter, you'll be prompted for it. If you enter invalid input as you define it for your parameters, it will spit out an error and make you try again.