Help with improving Lync script

PowerShell Help
1

I have been working on this script for a few days now based on my limited PowerShell skills and I would welcome some feedback from some of the more experienced of you on where I can improve the script and what things I have done wrong or not to best practise so I can correct them where necessary.

$EUC_OU = “OU=MyOU,DC=my,DC=domain,DC=com”
$FEPool = “fepool1.mydomain.com”
$LyncGroup = “GG-Lync-Deny-Access”

$CountSkipped = [int]“0”
$CountEnabled = [int]“0”
$CountDisabled = [int]“0”
$CountProgress = [int]“0”

Function Test-ADGroupMember {
Param ($User,$Group)
Trap {Return “error”}
If (
Get-ADUser -Filter “memberOf -RecursiveMatch ‘$((Get-ADGroup $Group).DistinguishedName)’” -SearchBase $((Get-ADUser $User).DistinguishedName)
) {$true}
Else {$false}
}

#Retrieving a list of users in the OU to be Lync enabled
$CSUsers = Get-CsAduser -filter {WindowsEmailAddress -like “*@whitbread.com”} -OU $EUC_OU
$CountTotal = ($CSUsers | Measure).Count
ForEach($CSU in $CSUsers)
{
$CountProgress++
Write-Progress -activity “Processing list of users” -status "Percent complete: " -percentComplete (($CountProgress / $CountTotal) * 100)
#Get the variables in place for the rest of the script
$SamAccountName = $CSU.SamAccountName
$DisplayName = $CSU.DisplayName
$UPN = $CSU.UserPrincipalName
#Checking if the user is a member of the Lync deny security group
$LyncDeny = Test-ADGroupMember -User $SamAccountName -Group $LyncGroup

        if ($LyncDeny -ne $True)
                {
                    #If in here then user is not a member of the Lync deny group so check if they are already enabled and if not enable them
                    if ($CSU.Enabled -ne $True)
                    {
                        #Enabling the user for Lync
                        Enable-CsUser -Identity $UPN -RegistrarPool $FEPool -SipAddressType EmailAddress
                        $CountEnabled++
                        Write-Output "User $DisplayName has been enabled for Lync"
                    }
                    else
                    {
                    Write-Verbose "User $DisplayName is already enabled so skipping"
                    $CountSkipped++
                    }
                }
                else
                {
                    #If in here then user is a member of the Lync deny group so check if they are enabled and disable them
                    if ($CSU.Enabled -eq $True)
                    {
                        #User is enabled for Lync and should not be so will disable them on Lync
                        Write-Warning "User $DisplayName is enabled for Lync and shouldn't be ... commencing disabling"
                        Set-CsUser -Identity $UPN -Enabled $False
                        Disable-CsUser -Identity $UPN -Confirm:$false
                        $CountDisabled++

                    }
                    else
                    {
                        Write-Verbose "User $DisplayName is a member of the group $LyncGroup so not enabling for Lync"
                        $CountSkipped++
                    }                        
                }
        
        }

Write-Output "Results "
Write-Output “Total: $CountTotal”
Write-Output “Skipped: $CountSkipped”
Write-Output “Enabled: $CountEnabled”
Write-Output “Disabled: $CountDisabled”

2

Welcome to the Forum! For a beginner, your script looks good, but you are working a little hard. It’s better to filter as much as possible to return what you need versus trying to process it with logic. This looks very similar to what you are trying to do and notice that the LDAPFilter is filtering the group, if it’s Lync enabled, etc. all with Get-CSAdUser:

http://andywolf.com/powershell-script-to-enable-all-users-for-lync-and-disable-unauthorized-users/

Some other notes:

  1. Typically, scripts are ordered with CmdLetBinding, Params, Functions, variables, logic. You'll see this is the typical order as your read scripts.
  2. Your variable is a bit of an oxymoron. $CountEnabled = [int]"0" basically is casting a string into integer, which is just extra work. [int]$CountEnabled = 0 is a strongly-typed variable, but $CountEnabled = 0 is the same thing. You're just forcing a string into a integer when your are defining what the variable.
3

Thanks Rob for taking the time to read my post and give some feedback. The link you sent certainly does achieve a very similar result with a lot less effort!

I’ve been watching some Microsoft MVA videos with Jason Helmick and Jeffrey Snover which give information about functions, params etc so i’ll look into adding those bits as although the script works for me right now I was planning on sharing it with my colleagues and so wanted to make sure I’m doing it as per best practise etc to make it more usable for others.