Automatically remove O365 license from AD user when using powershell

Question

As you can see I have been using multiple conditions such as if, elseif, else, but it takes too much time to complete the process.

How can I improve the speed of the script?

Import-Module ActiveDirectory
Import-Module MSOnline

$password = ConvertTo-SecureString 'PASSWORD' -AsPlainText -Force
$LiveCred = New-Object System.Management.Automation.PSCredential ("[email protected]", $password)
New-PSSession -ConfigurationName Microsoft.Exchange -ConnectionUri https://ps.outlook.com/powershell/ -Credential $LiveCred -Authentication Basic -AllowRedirection

Connect-MsolService -Credential $Livecred



$userID = Import-Csv "c:\export\list.csv"
$LogFile = "C:\export\logs.txt"
foreach ($user in $userID)
{

    $ADuser = Get-ADUser -Filter "EmployeeId -eq $($user.EmployeeID)" -Properties whenCreated, Enabled, SAMAccountName
    $O365User = Get-MsolUser -UserPrincipalName $ADuser.UserPrincipalName

    if (($ADuser.Enabled -eq $true) -and ($O365User.isLicensed -eq $true))
    {
        Get-MsolUSer -UserPrincipalName $ADuser.UserPrincipalName
        Set-MsolUserLicense -UserPrincipalName $ADuser.UserPrincipalName -RemoveLicenses "company:ENTERPRISEPACK"
        #move user OU, change description, disable account, remove SG members list
        #$Date = Get-Date -Format MM-dd-yyyy
        #Set-ADUser -Identity $ADuser.SAMAccountName -Replace @{info="User disabled at $Date"}
        # $User has been disabled and remove office 365 licence from user.
        #"$ADuser.SAMAccountName has been disabled and remove office 365 licence from user." | Out-File $LogFile -Append -Force

    }
    elseif (($ADuser.Enabled -eq $true) -and ($O365User.isLicensed -eq $false))
    {
        #move user OU, change description, disable account, remove SG members list
        #Set-ADUser username -Replace @{info='New info for the notes field'}
        #"$ADuser.SAMAccountName has been disabled and already without office 365 licence." | Out-File $LogFile -Append -Force

    }
    elseif (($ADuser.Enabled -eq $false) -and ($O365User.isLicensed -eq $false))
    {
        #do nothing
        #"$ADuser.SAMAccountName has been already disabled and already without office 365 licence." | Out-File $LogFile -Append -Force

    }
    elseif (($ADuser.Enabled -eq $false) -and ($O365User.isLicensed -eq $true))
    {
        Get-MsolUSer -UserPrincipalName $ADuser.UserPrincipalName
        Set-MsolUserLicense -UserPrincipalName $ADuser.UserPrincipalName -RemoveLicenses "company:ENTERPRISEPACK"
        #"$ADuser.SAMAccountName has been already disabled and but revoke office 365 licence." | Out-File $LogFile -Append -Force

    }
    else
    {
        #User does not exist in AD
        #"$user.EmployeeID does not exist in Active Directory." | Out-File $LogFile -Append -Force

    }


}

Answer 1

Not entirely sure if the code you have posted is exactly what you want reviewed but I am going to write this based on what I see.

Clear text passwords

Have to cover this one first to get it out of the way. I see that you have hardcoded a password. This is a bad practice of security though obscurity. Anyone opening the script via text will be able to see your credential set. While this is not always ideal I would suggest that you have your script prompt the user for credentials using Get-Credential.

Empty if clauses

As you can see I have been using multiple conditions such as if, elseif, else

Yes, I can see that. However most of these blocks are empty with comments stating "Do nothing".

elseif (($ADuser.Enabled -eq $false) -and ($O365User.isLicensed -eq $false))
{
    #do nothing
    #"$ADuser.SAMAccountName has been already disabled and already without office 365 licence." | Out-File $LogFile -Append -Force

}

Why bother wasting CPU cycles on something that does not matter? I assume at one point you had code in there but now you don't so just remove those empty clauses.

Repeating code

At the start of your loop pass you have a call to Get-MsolUser.

$O365User = Get-MsolUser -UserPrincipalName $ADuser.UserPrincipalName

Then inside your loops you make that call again but it just go to standard output. Why bother getting the same information again? Nothing would have changed in that time. I would just removed those lines in your if clauses. If you wanted to see it on screen you could just write the line $O365User and that would save calling the cmdlet again.

Reducing if complexity

So of the remaining if clauses that acutally do something they are both doing the same thing! Removing a license via -RemoveLicenses "company:ENTERPRISEPACK". You seem to be doing this regardless of the user being enabled or not.

This is another part of the code that make me wonder if this is what you're actually doing or if you scrubbed it too much for this question. Either way I am taking if for face value here.

Instead of checking whether the user is enabled or not simply filter for $O365User.isLicensed and process those.

$userID = Import-Csv "c:\export\list.csv"
$LogFile = "C:\export\logs.txt"
foreach ($user in $userID)
{
    $ADuser = Get-ADUser -Filter "EmployeeId -eq $($user.EmployeeID)" -Properties whenCreated, Enabled, SAMAccountName
    $O365User = Get-MsolUser -UserPrincipalName $ADuser.UserPrincipalName
    if ($O365User.isLicensed){Set-MsolUserLicense -UserPrincipalName $ADuser.UserPrincipalName -RemoveLicenses "company:ENTERPRISEPACK"}
}

Error prevention

In your code and my snippet there is not error prevention built in. If there was a bad entry in the CSV or something along those lines the script would fail if it did not find a user. Consider using -ErrorAction SilentlyContinue and then checking if the variables contain data.

$ADuser = Get-ADUser -Filter "EmployeeId -eq $($user.EmployeeID)" -ErrorAction SilentlyContinue
if($ADUser){
    # Process as normal
} else {
    Write-Warning "No matching user for $($user.EmployeeID) was located."
}

You will notice that I dropped the properties parameter from your Get-ADUser call. Enabled and samaccountname are returned by default. While whenCreated is not, you don't use it either. True, the return in performance is likely not measurable - but if you don't use it just remove it.

Jobs and workflows

This can be a large subject so I will touch on it here. If you have 1000's of these entries to process you might want to have a look at jobs or parallel processing:

• https://stackoverflow.com/questions/4016451/can-powershell-run-commands-in-parallel

• https://stackoverflow.com/questions/8781666/run-n-parallel-jobs-in-powershell

• about_foreach_parallel