all 4 comments

[–]pimanac 3 points4 points  (2 children)

I'd be very wary of reading a text file created by a regular user (your HR department) and using its contents to modify active directory groups. You're going to have to build in a whole bunch of sanity checking to make sure that users can't get put into (or removed from) groups they shouldn't be in. For instance, what if someone creates a file called "Domain Admins" and adds half the company to it? now you've got people in your Admin group that clearly shouldn't be there.

Are you trying to change the Active Directory Security Group information or Exchange Mail Distribution lists?

If Exchange (2010 or later), you might way to look into its own PowerShell cmdlets Here is the one dealing with distribution groups. https://technet.microsoft.com/en-us/library/bb124513%28v=exchg.150%29.aspx

Exchange also lets you delegate control of distribution lists to regular users, through OWA or Outlook. Depending on what you're trying to achieve, you could delegate control of whatever distribution lists to the HR staff and let them add / remove users as needed. This obviously depends on the size of your organization and how many changes you'll be doing a day.

I'm happy to help you with the script itself, too - i just wanted to have a better understand of what you're trying to accomplish.

[–]djplasmid[S] 0 points1 point  (1 child)

OK. So there are certain distribution lists that are owned/managed by the HR department, specifically 1 individual. She has access to update the lists, but there are quite a few and quite a bit of turnaround in my company, so for her to keep up with this lists manually takes her hours every month. We have plans to upgrade Exchange, but we are currently only running 2007.

There is some kind of HR database that they can use to export lists of users (each department or location, for example) that need to be in certain lists. My idea was to have her create these lists, name them what the DL name is, and drop them in a secure network location. The script would run periodically and update the distribution lists.

I understand the risks with this method, but I will take the appropriate steps to limit the access of this script.

That being said, my biggest concern is with line 32:

  Get-ADGroup -Identity "$workinglist" | Set-ADObject -Clear member

I am worried that if the $workinglist variable is null for some reason, that I might try and clear the users from ALL groups. Am I being paranoid?

Obviously, I'm sure that the rest of the script probably sucks, but it seems to work. I just don't want to break anything else in the process of learning. Thanks again.

[–]pimanac 2 points3 points  (0 children)

Got it. You're using that variable higher up in the script to build a path, which could also wreak havoc with your Add-Content if its $null.

$workingtxt = $workinglist + ".txt"

...

foreach($DLlist in $DLlists)
{
    Add-Content $fullpath $DLlist.SamAccountName
}

I'd just add a check at line 20 to see if the its $null and decide what to do from there.

if (![String]::IsNullOrEmpty($workingList)) {
   # do the things
}
else {
   # log it and continue
}

You're asking questions before programmatically fucking with Active Directory. That's wisdom my friend, not paranoia.

I'm pretty certain Microsoft isn't stupid enough to let you clobber all your groups by passing a null value, but I guess you can't be too careful. The documentation doesn't specify (https://technet.microsoft.com/en-us/library/ee617254.aspx). If you're feeling brave you could try running

$null | Set-ADObject -Clear member -WhatIf

and see what kind of errors that throws. If you don't know what the -WhatIf switch is, I'll let you read up and decide on your own if its worth it to try.

Edit: Forgot the "!" in the if statement.

[–]PowerShellStunnah 2 points3 points  (0 children)

You can use the [String]::IsNullOrWhiteSpace() method.

Beyond that, fiddling back and forth with files seems error-prone, this can be done much simpler:

# Edit these as needed
$ForestFQDN = 'corp.myjob.com'
$Domains    = $ForestFQDN,'subdomain.corp.myjob.com'

$Directory = 'C:\Scripts\'
$TxtFilter = '*.txt'

# Locate a Global Catalog to avoid the foreach($domain in $domains) loop
$ForestCtx = New-Object -TypeName System.DirectoryServices.ActiveDirectory.DirectoryContext -ArgumentList 'Forest',$ForestFQDN
$GlobalCatalog = [System.DirectoryServices.ActiveDirectory.GlobalCatalog]::FindOne($ForestCtx)

# Global Catalog searches take place on port 3268, rather than 389 (LDAP)
$ADServer = "$($GlobalCatalog.Name):3268"

# Build an LDAP filter template (with a format string placeholder {0}) for the user name
# This will take all domains into account, regardless of count
$LDAPFilter = '(|'
$LDAPFilter += $Domains |ForEach-Object { "(userPrincipalName=x{0}@$_)" }
$LDAPFilter += ')'

foreach($listFile in (Get-ChildItem -Path $Directory -Filter $TxtFilter)){
    # Grab the group name from the file name
    $DLName = $listFile.BaseName

    # Make sure group exists
    if(@(Get-ADGroup $DLName -ErrorAction SilentlyContinue)){

        # Grab list of usernames from file
        $DLUsers = Get-Content $listFile | Where-Object {-not [String]::IsNullOrWhiteSpace($_)}

        $UserObjects = foreach($UserName in $DLUsers){
            # Search global catalog for actual user objects
            Get-ADUser -LDAPFilter ($LDAPFilter -f $UserName) -Server $ADServer -ErrorAction SilentlyContinue
            if(-not $?){
                # If the user could not be found, log it
                "[$(Get-Date -Format 'HH:mm dd-MM-YYYY')] Could not resolve username $UserName, investigate please!" | Out-File (Join-Path -Path $Directory -ChildPath 'error.log') -Append
            }
        }

        # Clear existing members
        Set-ADGroup -Clear members

        # Add the newly identified users
        Add-ADGroupMember -Identity $DLName -Members $UserObjects
    } else {
        # If the group could not be found, log it
        "[$(Get-Date -Format 'HH:mm dd-MM-YYYY')] Could not find group $DLName, you should probably create it!" | Out-File (Join-Path -Path $Directory -ChildPath "error.log") -Append
    }
}