Home > Software design >  Trying to cleanup a PS script I made
Trying to cleanup a PS script I made

Time:11-04

I work for a k-12 district and I've been writing a script to add students in after school activities to different AD security groups so I can apply access card access specifically to their later schedules (currently all students have a standard time and we've just been leaving doors unlocked for after school stuff).

Our students' after school activities are rostered in our SIS and Azure can pull all of those rosters and apply them to users. Our Access control manager only syncs with AD and not azure so I've made AD security groups and this script is attempting to sync the azure groups over to the security groups. I'm fairly new to powershell, so while I do have it working, my script is very long and took forever to figure out. I'm looking for reference on how I can shorten it

$VB =  Get-AzureADGroupMember -ObjectId zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz  #volleybally
$SOB = Get-AzureADGroupMember -ObjectId zzzzzzzzzzzzzzzzzzzzzzzzzzzz  #Boys Soccer
$SOG = Get-AzureADGroupMember -ObjectId xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx5  #Girls Soccer
$BG = Get-AzureADGroupMember -ObjectId 7xxxxxxxxxxxxxxxxxxxxxxxxxxx   #Boys Golf
$GG = Get-AzureADGroupMember -ObjectId xxxxxxxxxxxxxxxxxxxxxxxxxxxxx1c   #Girls Golf
$CC = Get-AzureADGroupMember -ObjectId zzzzzzzzzzzzzzzzzzzzzzzzzzzzz  #Cross Country
$FCL = Get-AzureADGroupMember -ObjectId xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx  #Fall Cheerleading
$SDD = Get-AzureADGroupMember -ObjectId xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx #Speech Drama &  Debate


#initiate SG Groups

$SGCC = Get-ADGroup -Identity sample group 1
$SGFC = Get-ADGroup -Identity sample group 2
$SGFB = Get-ADGroup -Identity sample group 3
$SGGO = Get-ADGroup -Identity sample group 4
$SGSO = Get-ADGroup -Identity sample group 5
$SGVB = Get-ADGroup -Identity sample group 6


# convert azure values to AD and add to AD groups

$FB= Foreach($ad in $FB) {
    Get-ADUser ($ad.UserPrincipalName.Split('@')[0])
}

$VB = Foreach($ad in $VB) {
    Get-ADUser ($ad.UserPrincipalName.Split('@')[0])
    # your text
}

$SOB = Foreach($ad in $SOB) {
    Get-ADUser ($ad.UserPrincipalName.Split('@')[0])
}

$SOG = Foreach($ad in $SOG) {
    Get-ADUser ($ad.UserPrincipalName.Split('@')[0])
}

$BG = Foreach($ad in $BG) {
    Get-ADUser ($ad.UserPrincipalName.Split('@')[0])
}

$GG = Foreach($ad in $GG) {
    Get-ADUser ($ad.UserPrincipalName.Split('@')[0])
}
$CC = Foreach($ad in $CC) {
    Get-ADUser ($ad.UserPrincipalName.Split('@')[0])
}

$FCL = Foreach($ad in $FCL) {
    Get-ADUser ($ad.UserPrincipalName.Split('@')[0])
}

$SDD = Foreach($ad in $SDD) {
    Get-ADUser ($ad.UserPrincipalName.Split('@')[0])
}

#### add AD Members to SG

Add-AdGroupMember -Identity $SGFB -Members $FB
# Looking to simplify, To be continued

I've written it the long way. Curious if anyone has ideas on how to do it better. Like I said, I'm a PS noob.

CodePudding user response:

I think you could simplify your task by using a hash table ($map) where the Keys are the GUIDs of each Azure Group and the Values are each AD Group where the Az Group members need to be added.

For example:

$map = @{
    'xxxxxxxxxxxx' = 'group 1', 'group 5', 'group 8' # Football
    'zzzzzzzzzzzz' = 'group 2'                       # Volleyball
    'yyyyyyyyyyyy' = 'group 3', 'group 4'            # Boys Soccer
    # and so on here
}

foreach($pair in $map.GetEnumerator()) {
    # I think you could use `DisplayName` instead of `UserPrincipalName` here
    # and don't have a need to parse the UPNs
    $azMembers = (Get-AzureADGroupMember -ObjectId $pair.Key).DisplayName
    foreach($adGroup in $pair.Value) {
        Add-ADGroupMember -Identity $adGroup -Members $azMembers.DisplayName
    }
}

As stated in the inline comment, I believe using .DisplayName would suffice since -Members takes one of these values:

  • Distinguished name
  • GUID (objectGUID)
  • Security identifier (objectSid)
  • SAM account name (sAMAccountName)

But considering this may not be case and that doesn't work, then an easier and safer way to parse the user's UserPrincipalName rather than using .Split('@')[0] would be to use the MailAddress class, so using it the code would look like this:

# here goes the `$map` too!

foreach($pair in $map.GetEnumerator()) {
    $azMembers = [mailaddress[]] (Get-AzureADGroupMember -ObjectId $pair.Key).UserPrincipalName
    foreach($adGroup in $pair.Value) {
        Add-ADGroupMember -Identity $adGroup -Members $azMembers.User
    }
}
  • Related