🧹 chore: code quality audit — fix critical, high, medium, and low bugs

Active Directory/AD Groups/Archive-ObsoleteGroups.ps1

@@ -21,11 +21,23 @@
 #Import the Active Directory module so we can work with AD groups.
 Import-Module ActiveDirectory
 
-#Set the Active Directory server name that will be used. Using a serverless domain name here may also work.
-$Domain = ''
+# TODO: This script requires customization before running. The $Domain, archive paths,
+# TODO: group identity/OU values in the loop, and Move-ADObject target path must all be set
+# TODO: for your environment. See inline TODO comments below.
+param (
+    # The Active Directory domain controller or domain name to run against.
+    [Parameter(Mandatory)]
+    [string]
+    $Domain,
+
+    # Path to the file listing obsolete group names (one per line).
+    [Parameter()]
+    [string]
+    $GroupListPath = 'C:\Scripts\ObsoleteGroups\ObsoleteGroups.csv'
+)
 
 #Read in the CSV or text file of group names.
-$File = Get-Content -Path C:\Scripts\ObsoleteGroups\ObsoleteGroups.csv
+$File = Get-Content -Path $GroupListPath
 
 #Loop through each line of the text file and run the following commands for each line:
 Foreach ($Group in $File) {
@@ -37,11 +49,13 @@
         This section will require special customization until we further develop the script to pull the full group DN.
         In the interest of time today, I have hard coded some of the information.
         * * * * * * * * * *
-    /#>
+    #>
+    # TODO: Replace the -group, -ou, and -domain placeholder values with real values for your environment.
     .\Remove-AllGroupMembers.ps1 -group "CN=$Group" -ou 'OU=' -domain 'DC='
+    # TODO: Replace -Identity and -TargetPath with the correct DN values for your environment.
     Move-ADObject -Server $Domain -Identity 'CN=ps,DC=' -TargetPath ''
 }
 
 #Copy and rename the CSV file with a timestamp to keep as a record of run history.
-$timeStamp = Get-Date -Format 'yyyy-MM-dd hh-m-ss'
-Copy-Item -Path C:\Scripts\ObsoleteGroups\ObsoleteGroups.csv -Destination "C:\Scripts\ObsoleteGroups\Run History\ObsoleteGroups $timeStamp.csv"
+$timeStamp = Get-Date -Format 'yyyy-MM-dd HH-mm-ss'
+Copy-Item -Path $GroupListPath -Destination "C:\Scripts\ObsoleteGroups\Run History\ObsoleteGroups $timeStamp.csv"

Active Directory/AD Users/Get-InactiveADUser.ps1

@@ -96,7 +96,7 @@
 
         if ($CheckAllDCs) {
             # Skip the check across all DCs if there is already a LastLogonDate within the past 14 days and if the most recent logon is more recent than the inactive date threshold.
-            if ( $MostRecentLogon -lt (Get-Date).AddDays(-14) -and (-not $MostRecentLogon -lt $InactiveDate) ) {
+            if ( $MostRecentLogon -lt (Get-Date).AddDays(-14) -and (-not ($MostRecentLogon -lt $InactiveDate)) ) {
                     # Check LastLogon (non-replicated) on every domain controller.
                     foreach ($DC in $DomainControllers) {
                         try {
@@ -143,7 +143,7 @@
 
         # Optional: Export to CSV
         if ($PSBoundParameters.ContainsKey('ExportCSV')) {
-            $ExportPath = ".\InactiveUsers_$(Get-Date -Format 'yyyyMMdd_HHmmss').csv"
+            $ExportPath = if ($ExportCSV) { $ExportCSV } else { ".\InactiveUsers_$(Get-Date -Format 'yyyyMMdd_HHmmss').csv" }
             $Results | Export-Csv -Path $ExportPath -NoTypeInformation
             Write-Host "Results exported to: $ExportPath" -ForegroundColor Green
         }

Active Directory/AD Users/Get-InactiveUsers.ps1

@@ -73,7 +73,7 @@
         Write-Verbose 'Active Directory module imported successfully'
     } catch {
         Write-Error "Failed to import Active Directory module: $($_.Exception.Message)"
-        break 1
+        return
     }
 
     # Calculate the cutoff date and its file time representation for the AD filter.
@@ -99,7 +99,7 @@
         Write-Host "Found $($InactiveUsersResult.Count) inactive user account(s)." -ForegroundColor Green
     } catch {
         Write-Error "Failed to get user accounts: $($_.Exception.Message)"
-        break 1
+        return
     }
 
     $InactiveUsers = @()

Active Directory/AD Users/Get-LockedOutLocation.ps1

@@ -48,7 +48,7 @@
             $DCCounter++
             Write-Progress -Activity 'Contacting DCs for lockout info' -Status "Querying $($DC.Hostname)" -PercentComplete (($DCCounter / $DomainControllers.Count) * 100)
             try {
-                $UserInfo = Get-ADUser -Identity $Identity -Server $DC.Hostname -Properties AccountLockoutTime, LastBadPasswordAttempt, BadPwdCount, LockedOut -ErrorAction Stop
+                $UserInfo = Get-ADUser -Identity $Identity -Server $DC.Hostname -Properties AccountLockoutTime, BadPasswordTime, LastBadPasswordAttempt, BadPwdCount, LockedOut -ErrorAction Stop
             } catch {
                 Write-Warning $_
                 continue
@@ -74,7 +74,7 @@
             $LockedOutEvents = Get-WinEvent -ComputerName $PDCEmulator.HostName -FilterHashtable @{LogName = 'Security'; Id = 4740 } -ErrorAction Stop | Sort-Object -Property TimeCreated -Descending
         } catch {
             Write-Warning $_
-            continue
+            return
         }#end catch
 
         foreach ($item in $LockedOutEvents) {

Active Directory/Domain Services/Get-FSMORoleDetails.ps1

@@ -1,8 +1,16 @@
 #WIP
 
 # Get the hostname and AD site location of domain controllers that hold the AD FSMO roles.
-$FSMORoles = Get-ADForest | Select-Object -ExpandProperty SchemaMaster, DomainNamingMaster
-$FSMORoles += Get-ADDomain | Select-Object -ExpandProperty PDCEmulator, RIDMaster, InfrastructureMaster
+# Note: -ExpandProperty only accepts one property at a time; collect each role separately.
+$Forest = Get-ADForest
+$Domain = Get-ADDomain
+$FSMORoles = @(
+    $Forest.SchemaMaster
+    $Forest.DomainNamingMaster
+    $Domain.PDCEmulator
+    $Domain.RIDMaster
+    $Domain.InfrastructureMaster
+)
 
 # Get the details of each FSMO role holder
 foreach ($role in $FSMORoles) {

Active Directory/Export-AllADUserTransitiveGroupMemberships.ps1

@@ -52,7 +52,7 @@ begin {
                         Get-ADUserTransitiveGroupMembership -UserDN $_.DistinguishedName
                     }
                 }
-            Write-Verbose -Message "  - Found $($UserCount) users in the domain."
+            Write-Verbose -Message "  - Found $($Users.Count) users in the domain."
 
             # Export the data to a JSON file.
             $JsonData = $Users | ConvertTo-Json
@@ -148,7 +148,7 @@ begin {
             }
 
             $CurrentProgressPreference = Get-Variable -Name ProgressPreference -ValueOnly
-            Set-Variable -Name ProgressPreference 'SilentlyContinue' -Scope Global -Force -ErrorAction SilentlyContinue
+            Set-Variable -Name ProgressPreference -Value 'SilentlyContinue' -Scope Global -Force -ErrorAction SilentlyContinue
             # Check if the global catalog server is available on the specified port.
             if (-not (Test-NetConnection -ComputerName $Server -Port $Port -InformationLevel Quiet -ErrorAction SilentlyContinue)) {
                 if (-not (Test-NetConnection -ComputerName $Server -Port $AltPort -InformationLevel Quiet -ErrorAction SilentlyContinue)) {

Active Directory/Export-AllGroupMemberships.ps1

@@ -30,7 +30,7 @@ function Export-AllUserGroupMemberships {
     process {
         # Get all users in the domain and their group memberships.
         Write-Verbose -Message 'Getting all enabled users in the domain.'
-        $Users = Get-ADUser -Filter 'Enabled -eq $true' -Properties EmployeeId, memberOf |
+        $Users = Get-ADUser -Filter { Enabled -eq $true } -Properties EmployeeId, memberOf |
             Select-Object Name, DisplayName, samAccountName, userPrincipalName, EmployeeId, memberOf
         Write-Verbose -Message "  - Found $($Users.Count) users in the domain."
 

Active Directory/Get-ADAttributeUniqueValues.ps1

@@ -47,7 +47,7 @@ function Get-ADAttributeUniqueValues {
         [ValidateNotNullOrEmpty()]
         [ValidateSet('company', 'country', 'department', 'homeDrive', 'l', 'physicalDeliveryOfficeName', 'postalCode', 'state', 'streetAddress', 'title')]
         [string[]]
-        $AttributesToCheck = @('Company', 'Department', 'Office', 'Title'),
+        $AttributesToCheck = @('company', 'department', 'physicalDeliveryOfficeName', 'title'),
 
         # The directory to save the exported JSON file in. (Optional, defaults to C:\Temp.)
         [Parameter()]

Active Directory/Get-ADObjectFromPipeline.ps1

@@ -16,7 +16,10 @@ function Get-ADObjectFromPipeline {
     begin {
         Import-Module ActiveDirectory
         $GlobalCatalog = Get-ADDomainController -Discover -Service GlobalCatalog
+    }
 
+    process {
+        # Resolve identity type in process block where pipeline input ($Identity) is available.
         if ($Identity -is [Microsoft.ActiveDirectory.Management.ADUser]) {
             # We have an ADUser object
             # Might want to normalize the type to an ADObject IF we can get sidHistory from an ADObject
@@ -30,9 +33,6 @@ function Get-ADObjectFromPipeline {
             $Identity = Get-ADObject -Filter "Name -eq `"$Identity`""
         }
         $IdentityType = $Identity.ObjectClass
-    }
-
-    process {
         switch ($IdentityType) {
             'user' {
                 # Not Complete

Active Directory/Get-ADUserTransitiveGroupMembership.ps1

@@ -57,7 +57,7 @@ function Get-ADUserTransitiveGroupMembership {
         }
 
         $CurrentProgressPreference = Get-Variable -Name ProgressPreference -ValueOnly
-        Set-Variable -Name ProgressPreference 'SilentlyContinue' -Force -Scope Global -ErrorAction SilentlyContinue
+        Set-Variable -Name ProgressPreference -Value 'SilentlyContinue' -Force -Scope Global -ErrorAction SilentlyContinue
         # Check if the global catalog server is available on the specified port.
         if (-not (Test-NetConnection -ComputerName $Server -Port $Port -InformationLevel Quiet -ErrorAction SilentlyContinue)) {
             if (-not (Test-NetConnection -ComputerName $Server -Port $AltPort -InformationLevel Quiet -ErrorAction SilentlyContinue)) {
@@ -80,10 +80,11 @@ function Get-ADUserTransitiveGroupMembership {
         $TransitiveMemberOfGroupDNs = foreach ($result in ($results.properties)) {
             $result['distinguishedname']
         }
+        # Emit deduplicated results per user in process block so pipeline results are not overwritten.
+        $TransitiveMemberOfGroupDNs | Sort-Object -Unique
     }
 
     end {
-        $TransitiveMemberOfGroupDNs | Sort-Object -Unique
         Remove-Variable Filter, TransitiveMemberOfGroupDNs, Results, Searcher, Server, Port, UserDN -ErrorAction SilentlyContinue
     }
 } # end function Get-ADUserTransitiveGroupMembership

DDI/Get Hostnames from CSV IP Addresses.ps1

@@ -10,7 +10,7 @@ $IPAddressList | foreach-object {
         $_.Hostname = ([System.Net.Dns]::GetHostEntry($ip)).HostName
     }
     catch {
-        Write-Error $error[0] #.Exception.Message.Split(':')[1]
+        Write-Error $_ #.Exception.Message.Split(':')[1]
     }
 }
 # Write the data back to the CSV with the hostnames added.

DDI/Update-DnsServerList.ps1

@@ -20,7 +20,7 @@ function Update-DnsServerList {
     foreach ($netadapter in $NetworkAdapters) {
         [ipaddress[]]$ClientDnsServerSearchOrder = $netadapter.DnsServerSearchOrder
         if (Compare-Object -ReferenceObject $ClientDnsServerSearchOrder -DifferenceObject $OldDnsServers -IncludeEqual -ExcludeDifferent) {
-            $NetAdapterConfig = Get-CimInstance -Class Win32_NetworkAdapterConfiguration -Filter "Index = $._Index"
+            $NetAdapterConfig = Get-CimInstance -Class Win32_NetworkAdapterConfiguration -Filter "Index = $($netadapter.Index)"
             $NetAdapterConfig.SetDnsServerSearchOrder($($NewDnsServers.IPAddressToString -join ','))
 
             IpConfig /FlushDns

Exchange/Parse-TransportLogs.ps1

@@ -13,10 +13,9 @@
   http://blog.chrislehr.com/2015/07/parse-transportlogs-which-ips-on-my.html
 #>
 
-Set-ExecutionPolicy RemoteSigned
 $ExchangeCredential = Get-Credential -Message "Please enter credentials to connect to your Exchange Server. `nThis will be used to pull message subject lines from the tracking logs."
 $ExchangeServer = Read-Host 'Please specify an Exchange Server name.'
-$ExchangeSession = New-PSSession -ConfigurationName Microsoft.Exchange -ConnectionUri http://$ExchangeServer.DOMAINNAME.org/PowerShell/ -Authentication Kerberos -Credential $ExchangeCredential
+$ExchangeSession = New-PSSession -ConfigurationName Microsoft.Exchange -ConnectionUri "http://$ExchangeServer/PowerShell/" -Authentication Kerberos -Credential $ExchangeCredential
 Import-PSSession $ExchangeSession -DisableNameChecking
 
 $SMTPLogPath = Read-Host "`nWhat is the path of the folder containing your SMTP transport logs?"
@@ -47,13 +46,20 @@
     $testProgress++; $testPercent = [math]::Round(($testProgress / $testCount), 2) * 100
     Write-Progress -Activity "Parsing message $testProgress of $testCount." -Status "$testPercent% complete" -PercentComplete $testPercent
 
-    $data = ($item.data.split('<')[1]).Split('>')[0]
+    $splitData = $item.data.Split('<')
+    if ($splitData.Count -lt 2) {
+        Write-Warning "Skipping item with no message-id delimiter: $($item.data)"
+        continue
+    }
+    $data = $splitData[1].Split('>')[0]
     $subject = (Get-MessageTrackingLog -MessageId $data).MessageSubject | Select-Object -Unique
     $item.MessageID = $data
     $item.Subject = $subject
-    $ErrorActionPreference = 'SilentlyContinue' #To avoid ambiguous error output if/when a hostname is not found.
-    $item.Hostname = ([System.Net.DNS]::GetHostbyAddress($item.IPAddress)).Hostname
-    $ErrorActionPreference = 'Continue'
+    try {
+        $item.Hostname = ([System.Net.DNS]::GetHostbyAddress($item.IPAddress)).Hostname
+    } catch {
+        # Hostname not found for this IP; leave blank.
+    }
 }
 
 Remove-PSSession $ExchangeSession.Id

General/Purge-InstalledModules.ps1

@@ -18,7 +18,7 @@ $Modules = @(
     'Az.MySql',
     'Az.Network',
     'Az.Nginx',
-    'Az.RedisCache'
+    'Az.RedisCache',
     'Az.Sql',
     'Az.SqlVirtualMachine',
     'Az.StackHCI',

General/Update-ModuleVersion.ps1

@@ -4,6 +4,7 @@
     param (
 
         # Specify the version to update from (or read from a module manifest).
+        [Parameter(Mandatory)]
         [version] $InputVersion,
 
         # Basic version switches.
@@ -86,7 +87,7 @@
         }
 
         if ("$NewVersion$PrereleaseTag" -notmatch $PatternValidation) {
-            Write-Error -Message "The prerelease version '$PrereleaseVersion' is not a valid semantic version." -ErrorAction Continue
+            Write-Error -Message "The prerelease version '$NewVersion$PrereleaseTag' is not a valid semantic version." -ErrorAction Continue
         } else {
             $matches | Write-Debug -Debug
             foreach ($match in $matches.GetEnumerator()) {

Windows/Activate and Get License.ps1

@@ -29,12 +29,10 @@ if (-not (Test-Path -Path $registryPath)) {
 
 # Add read permissions for SID (S-1-1-0, Everyone) to the registry key with inheritance
 $acl = Get-Acl -Path $registryPath
-$ruleSID = New-Object System.Security.AccessControl.RegistryAccessRule($sid, 'FullControl', 'ContainerInherit,ObjectInherit', 'None', 'Allow')
+$ruleSID = New-Object System.Security.AccessControl.RegistryAccessRule($sid, 'ReadKey', 'ContainerInherit,ObjectInherit', 'None', 'Allow')
 $acl.AddAccessRule($ruleSID)
 Set-Acl -Path $registryPath -AclObject $acl
-Write-Output "Added 'Interactive' group and SID ($sid) with read permissions (with inheritance) to the registry key."
+Write-Output "Added 'Interactive' group and SID ($sid) with read (ReadKey) permissions (with inheritance) to the registry key."
 
 #Remove the # below to make sure it will kick off the scheduled task on already enrolled devices
 Start-Process "$env:SystemRoot\system32\ClipRenew.exe"
-
-$ProductKey = (Get-CimInstance -ClassName SoftwareLicensingService).OA3xOriginalProductKey

Windows/Push-DNSClientServerAddresses.ps1

@@ -1,28 +1,47 @@
+[CmdletBinding()]
+param (
+    # The OU or container in Active Directory to search for servers.
+    [Parameter(Mandatory)]
+    [string] $SearchBase,
+
+    # The Active Directory domain controller to query.
+    [Parameter(Mandatory)]
+    [string] $ADServer,
+
+    # The DNS server addresses to assign to network adapters on the target servers.
+    [Parameter(Mandatory)]
+    [string[]] $DNSServerAddresses
+)
+
 Import-Module ActiveDirectory
-$servers = Get-ADComputer -SearchBase "" -Server "" -SearchScope Subtree -Filter *
+$servers = Get-ADComputer -SearchBase $SearchBase -Server $ADServer -SearchScope Subtree -Filter *
 foreach ($server in $servers)
 {
     # Connect to the server.
     $serverName = $server.Name
     Write-Output "Connecting to $serverName"
+    $s = $null
     try {
-        # Create and connect to the PSSession.
-        $s = New-PSSession -ComputerName $serverName
-        Enter-PSSession $s -ErrorAction SilentlyContinue
+        # Create the PSSession. Enter-PSSession is interactive-only and cannot redirect script commands remotely.
+        $s = New-PSSession -ComputerName $serverName -ErrorAction Stop
     }
     catch {
         # Log the failure and continue the for loop on the next item.
         Write-Output "Failed connection to $serverName"
         Continue
     }
 
-    # Connected to session. Now updated the DNS client server address on any interfaces that currently use a domain controller IP.
+    # Connected to session. Now update the DNS client server address on any interfaces that currently use a domain controller IP.
     try {
-        Get-NetIPInterface | Get-DnsClientServerAddress | Where-Object {$_.ServerAddresses -like '10.10.10.*'} | `
-        Set-DnsClientServerAddress -ServerAddresses ("","","") -Verbose
+        Invoke-Command -Session $s -ScriptBlock {
+            Get-NetIPInterface | Get-DnsClientServerAddress | Where-Object { $_.ServerAddresses -like '10.10.10.*' } |
+                Set-DnsClientServerAddress -ServerAddresses $using:DNSServerAddresses -Verbose
+        }
     }
     catch {
-        Write-Output "Failed to change the DNS client server address on $servername"
+        Write-Output "Failed to change the DNS client server address on $serverName"
+    }
+    finally {
+        if ($s) { Remove-PSSession -Session $s }
     }
-    Exit-PSSession
 } # End foreach server loop.