Who among us is a script guru? I'm working on a logon script.

[Ecrofirt]

OK, so I'm working on a VBS logon script for some users on our network.

The script checks if the users are members of specific groups, then maps various network drives if they are. It then runs another script at the end of everything.

I'm going to paste the script, and then I'm going to ask you a bunch of questions. I expect you to answer them IMMEDIATELY.

Code: [Select]


Option Explicit
Dim objShell 'This object represents the Windows Shell
Dim bResult 'This  represents the results of mapping drives

'This creates the shell object
Set objShell = WScript.CreateObject("WScript.Shell")

'Map the O:\ drive to \\fs01\odrive$ - This is done for all faculty and staff
If IsMemberOf("faculty-staff") = True Then
bResult = MapDrive("O:","\\fs01\odrive$")
End If

'Map the  L:\ drive to \\fs01\smg$  if the user is a member of the Sodexo_Menu_Graphics_Users group
If IsMemberOf("Sodexo_Menu_Graphics_Users") = True Then
bResult = MapDrive("L:","\\fs01\smg$")
End If

'Map the S:\ drive to \\fs01\chemdata$ if the user is a member of the chemdata_users group
If IsMemberOf("chemdata_users") = True Then
bResult = MapDrive("S:","\\fs01\chemdata$")
End If

'Map the I:\ drive to \\fs01\admshare$ if the user is a member of the Datatel-users group
If IsMemberOf("Datatel-users") = True Then
bResult = MapDrive("I:","\\fs01\admshare$")
End If

'Have the Windows shell execute Rick's update.bat file in a hidden window, and wait for the command to complete before continuing
objShell.Run "update.bat", 0, true


'Release the memory for the shell object
Set objShell = Nothing




'This function checks to see if the user is a member of the specified group
'Arguments:
' strGroupName - A string that represents a group name. EX: "faculty-staff"
'Returns:
' a boolean variable that corresponds to whether or not the user is a member of the specified group.
' If the user is a member of the group, 'true' is returned. Otherwise, 'false' is returned.
Function IsMemberOf(strGroupName)
dim objNetwork 'This will represent a WScript.Network object
dim bMember 'This is the boolean that corresponds to whether the user is a member of the group.
dim strDomain, strUserName 'These represent the current user's domain name and username
dim objUser 'This will be an object relating to the user on the network
dim objGroup 'This will be an object representing a Group on the network

'Create the network object
Set objNetwork = WScript.CreateObject("WScript.Network")

'Set the default value for whether the user is a member of the specified group
bMember = False


strDomain = objNetwork.UserDomain 'Retrieve the domain name
strUsername = objNetwork.UserName 'Retrieve the user's username

'Create the object that represents the user
Set objUser = GetObject("WinNT://" & strDomain & "/" & strUserName & ",user")

'This checks the group name of each group that the user is a member of.
'If the user is found to be a member of the sent group,  the bMember variable is set to true, and the for loop is exited.
For Each objGroup in objUser.Groups
'If the current group name matches the sent group
If objGroup.Name = strGroupName Then
'Set bMember to true, which represents that the user is a member of the group
bMember = True
'Exit the for loop
Exit For
End If
Next

Set objNetwork = Nothing 'Free the memory associated with the object
Set objUser = Nothing 'Free the memory associated with object

'Return the  value of the bMember variable
IsMemberOf = bMember

End Function

'This function attempts to map a UNC path to a network drive.
'Arguments:
' DriveLetter - This is a string that represents the drive letter that the UNC path should be mapped to, followed by a colon. EX: "O:"
' SharePath - This is a string that represents the UNC path to a network share. EX: "\\fs01\odrive$"
'Returns:
' a boolean variable that represents whether or not the drive mapping was a success.
' If the drive was successfully mapped, 'true' is returned. Otherwise, 'false' is returned
Function MapDrive(DriveLetter, SharePath)
On Error Resume Next 'continue with the script if an error happens

Dim objNetwork, objFileSystem, 'These will be objects that represent the Network, and the File System on the computer
Dim bSuccess 'A boolean variable that represents whether or not the mapping was a success

Set objNetwork = WScript.CreateObject("WScript.Network") 'Create the network object
Set objFileSystem = WScript.CreateObject("Scripting.FileSystemObject") 'Create the File System object

bSuccess = False 'Set the initial value of the Success variable to false, so that if an error occurs, the appropriate response is sent back

'This IF block checks 3 things:
'First, it checks to see if a drive currently exists at the drive letter that the UNC path is attempting to map to AND if the drive is a network drive
'If that fails, it checks to see if no drive currently exists at the desired drive letter
'Finally, it handles all other cases (for example, if a drive exists at the desired location, but it ISN'T a network drive)
'-----
'If a drive exists and it's a network drive
If (objFileSystem.DriveExists(DriveLetter) = True and objFileSystem.GetDrive(DriveLetter).DriveType = 3) Then
'WScript.Echo "deleting existing network drive " & DriveLetter & " and mapping a new one"
objNetwork.RemoveNetworkDrive DriveLetter 'Disconnect the network drive
objNetwork.MapNetworkDrive DriveLetter, SharePath 'Map the UNC path to the drive
bSuccess = true 'set the Success variable to true, indicating that the drive mapped
'If there's currently no drive at the desired letter
ElseIf (objFileSystem.DriveExists(DriveLetter) = False) Then
'WScript.Echo "Mapping network drive"
objNetwork.MapNetworkDrive DriveLetter, SharePath 'Map the UNC path to the drive
bSuccess = true 'set the Success variable to true, indicating that the drive mapped
'all other cases
Else
'WScript.Echo "Drive already exists! Drive Type is " & objFileSystem.GetDrive(DriveLetter).DriveType
End If


Set objNetwork = Nothing 'Free the memory used by the Network object
Set objFileSystem = Nothing 'Free the memory used by the FileSystem object

'Return the value of the Success variable
MapDrive = bSuccess

End Function



I hope you enjoy my commented code!

Now, here's my question. And I only ask this because I'm not 100% sure of the way some of these objects are returned.

Would the speed at which my script executes increase dramatically if I'm not constantly pulling in the user object and then comparing my group name to all groups associated with that object?

I'm thinking of doing something like this, but I don't know how it will increase efficiency:

Code: [Select]

Option Explicit
Dim objShell 'This object represents the Windows Shell
Dim bResult 'This  represents the results of mapping drives
Dim objUser        'This represents the user
'This creates the shell object
Set objShell = WScript.CreateObject("WScript.Shell")

objUser = GetUserObject()

'Map the O:\ drive to \\fs01\odrive$ - This is done for all faculty and staff
If IsMemberOf(objUser, "faculty-staff") = True Then
bResult = MapDrive("O:","\\fs01\odrive$")
End If


Function GetUserObject()
dim objNetwork 'This will represent a WScript.Network object
dim strDomain, strUserName 'These represent the current user's domain name and username
dim objUser 'This will be an object relating to the user on the network

'Create the network object
Set objNetwork = WScript.CreateObject("WScript.Network")


strDomain = objNetwork.UserDomain 'Retrieve the domain name
strUsername = objNetwork.UserName 'Retrieve the user's username

'Create the object that represents the user
Set objUser = GetObject("WinNT://" & strDomain & "/" & strUserName & ",user")

        GetUserObject = objUser

End Function


'This function checks to see if the user is a member of the specified group
'Arguments:
' strGroupName - A string that represents a group name. EX: "faculty-staff"
'Returns:
' a boolean variable that corresponds to whether or not the user is a member of the specified group.
' If the user is a member of the group, 'true' is returned. Otherwise, 'false' is returned.
Function IsMemberOf(objUser, strGroupName)
dim objNetwork 'This will represent a WScript.Network object
dim bMember 'This is the boolean that corresponds to whether the user is a member of the group.
dim objGroup 'This will be an object representing a Group on the network

'Create the network object
Set objNetwork = WScript.CreateObject("WScript.Network")

'This checks the group name of each group that the user is a member of.
'If the user is found to be a member of the sent group,  the bMember variable is set to true, and the for loop is exited.
For Each objGroup in objUser.Groups
'If the current group name matches the sent group
If objGroup.Name = strGroupName Then
'Set bMember to true, which represents that the user is a member of the group
bMember = True
'Exit the for loop
Exit For
End If
Next

Set objNetwork = Nothing 'Free the memory associated with the object
Set objUser = Nothing 'Free the memory associated with object

'Return the  value of the bMember variable
IsMemberOf = bMember

End Function       

That way I'm not pulling in the user object every time. Alternatively, I was going to look into pulling in the groups once (via a similar function called GetGroups or some shiz), and then changing IsMemberOf so that it took the groups object as well as the group name.


Can anyone comment on whether or not that will increase the speed of the script?

[Ecrofirt]

What, none of you have scripted before? Pussies.

[Van Cruncheon]

short answer: yes. pulling information from the ad is expensive. cache any values you know will recur (i.e. the user object) and assign them locally.

[Ecrofirt]

Short answer: Thanks drinky-poo.

I'm thinking of re-working it a bit so that it just pulls the groups into a local array, and when it iterates through the array when checking for memberships, it actually removes each element of the array as it's found.

[Ecrofirt]

I'm pretty excited that I get to tinker with a bit of programming at work more now with my new job, even if it's just basic stuff like this.

Our current domain is structured horribly, and the guy who's been doing the logon scripts for years has been of the philosophy where basically everyone gets a custom script. We've probably got 300 or so separate logon scripts for various users, even when they vary as slightly as 'this guy gets printers mapped because he requested them' and 'this staff member has asked for access to the g: drive'. Now I've got to wade through a million scripts that are all essentially the same just to try to clean things up a bit.

I'd have done things differently right from the start. I'd probably have more security groups in AD that related to specific things, and then I'd have a few tightly coded scripts that did everything on mass scale instead of the hundreds of scripts we have now.

[Van Cruncheon]

it's amazing how many it pro folks fear basic scripting and anything resembling code. if you're diving into it, then you've got a HUGE leg up on the competition.

[Ecrofirt]

I'll give you a huge leg up 

[TakingBackSunday]

Certainly not FoC

oh hoho

[recursivelyenumerable]

vbscript 
powershell

I know fuck all about AD, though.

[xnikki118x]

This is Ecrofirt:

I'd love to move more towards powershell. We're just starting to implement it here and there on campus, and it's not on every PC yet.

One of our students coded a nifty powershell script that queries all users with quota entries on a drive and send an email to us if the user is near their quota limit. Pretty fuck awesome.

[Ecrofirt]

OK, so I think I'm going to change around the code so that I create a local array of groups. I'm running into some issues.

Here's what it's doing:

Code: [Select]

Function GetGroups()

dim objNetwork 'This will represent a WScript.Network object
dim bMember 'This is the boolean that corresponds to whether the user is a member of the group.
dim strDomain, strUserName 'These represent the current user's domain name and username
dim objUser 'This will be an object relating to the user on the network
dim objGroup 'This will be an object representing a Group on the network
dim intNumGroups
dim curGroup 'This represents current group during the group iteration
Dim arrGroups()

'Create the network object
Set objNetwork = WScript.CreateObject("WScript.Network")

'Set the default value for whether the user is a member of the specified group
bMember = False


strDomain = objNetwork.UserDomain 'Retrieve the domain name
strUsername = objNetwork.UserName 'Retrieve the user's username

'Create the object that represents the user
Set objUser = GetObject("WinNT://" & strDomain & "/" & strUserName & ",user")

intNumGroups = (objUser.Groups.Count) - 1

ReDim arrGroups(intNumGroups)

curGroup = 0
For Each objGroup in objUser.Groups
arrGroups(curGroup) = objGroup.Name
curGroup = curGroup+1
Next

Set objUser = Nothing
set objNetwork = Nothing

GetGroups = arrGroups

End Function

I'm running into problems with the intNumGroups = (objUser.Groups.Count) - 1 line.

Apparently, I can't use Groups.Count. I'm sort-of scratching my head as to why, as it looks like I'm supposed to be able to. Here's my reference material:
http://msdn.microsoft.com/en-us/library/aa772211(VS.85).aspx - ADSI objects of WinNT
http://msdn.microsoft.com/en-us/library/aa746340(VS.85).aspx - IADsUser interface
http://msdn.microsoft.com/en-us/library/aa746342(VS.85).aspx - IADsUser::Groups Method
http://msdn.microsoft.com/en-us/library/aa706041(VS.85).aspx - IADsMembers Interface

Can one of you guys look that over very quickly and tell me if I'm barking up the wrong tree?

[xnikki118x]

Just bumping the thread.

[cubicle47b]

Fuck the array, just use a list and populate it with your for each loop.

[drozmight]

powershell scripting's awesome.

best thing ever MS.

[recursivelyenumerable]

Script gurus use vbscript? Undecided

Real script gurus can McGyver whatever's needed from whatever's available, I guess.

[xnikki118x]

Script gurus use vbscript? Undecided

Real script gurus can McGyver whatever's needed from whatever's available, I guess.

(bump)

He's asking for help!

EB

[Ecrofirt]

Why are you bumping my thread? I've already implemented all the needed changes and everything is working great. LOL.

[xnikki118x]

Why are you bumping my thread? I've already implemented all the needed changes and everything is working great. LOL.

Haha I didn't think you said that, lol.

Why not lock the thread then?

[Ecrofirt]

Perhaps the thread could have died a peaceful death if nosey nellies didn't go bumping it. Silly!

In any event, here's the script in its current form:

[cubicle47b]

Ew, resizing the array every time you add an element.  Again, you should just use a list.  Not that memory allocation and copying is going to take a large percentage of the execution time.

[Ecrofirt]

I had my reservations about resizing the array every time, and I still may not put it into the 'final' form of the script. It's just there to reduce the amount of shit to iterate through when checking member status.

I didn't think you could do lists in vbscript without coding your own class for them, though? That really starts to get away from the simplicity of a logon script.

[cubicle47b]

I never script using VBScript but arraylist should exist at the very least if generics don't.

edit: Hmm, arraylist is .Net 1.0.  I'm too lazy to look up what exists but my proposed solution for you if you have to use an array is to loop through the groups once to get the count, set the array size once, then loop through the groups again to set the array.  The other array option is to set a large size up front and then resize to the actual size of the array before you return.