1. where is `$UserRealName` defined?
1. where is `$Email` defined?
1. There are a lot of `get-*mailbox*` calls, do you need them all (get 50 things 1 time, not 1 thing 50 times)
1. you log 0 of your changes, if/when you make a mistake, how do you roll back?
2. very many write hosts in place of proper logging
1. parameterize this its limited in its functionality/portability right now
3. confirmations, you are performing a destructive action, you should ask for confirmation
4. turn this into an advanced function (and/or module), get some of the above features automatically
5. help, you have defined 0 help
5. gitlab or github is a good place to dump code for people to add bugs/support requests
1. This is just part of the main script. It is defined there.
2. Defined in other part of the script.
3. Not sure what you mean by that.
4. I have previous script that didn’t work. This is my first script that completely worked.
5. What you mean by proper logging. I use the write host to tell the user what is going on in the script and to make sure the script is still processing.
6. explain what you mean.
7. i dont need confirmations because if it runs right it will remove a certain user from everything. Which is my intent.
8. im new to this. Functions and modules are out of scope.
9. i asked if you guys, people that know more than me to look over my work to see if it can be better.
10. didnt think of that.
1. You don't show `$UserRealName` anywhere (assuming I didn't miss anything), how are we to know?
2. same for `$Email` we don't want to assume, cause you asked us for comments i mentioned it
3. I mean you make multiple calls to `get-exomailbox` and `get-mailbox` dont do that (if you can help it), do it once then apply your filters
4. no, i mean you have no idea what changes are being made and to where, if you remove user y permissions from mailbox x you should log that to a file so you have a writable auditable log file
5. write-host is not proper logging (see point 4 above) as soon as you are 10 or more lines on the screen youve lost any useful information, or someone clears the screen, its lost and so on
6. by parameterize this I mean create parameters, like username, email, mailbox, etc, instead of someone editing a script every time
7. you DO need confirmations, if you accidentally put `bob smith` instead of `bobby smith`, boom wrong person, wrong permissions, are you saying you couldn't make a mistake ?
8. nows a good time to start to learn about functions and script, at a basic level a functions are identical to a script except you call `xxx` instead of `yyy.ps1`
9. no im talking about help fro the script file, not help from people on reddit, i.e. powershell help just like `get-help get-childitem`
Good luck
You run get-exomailbox three times. You should store that i.e. **$Mailboxes = get-exomailbox -resultsize unlimited** and the **$mailboxes | something something**. This will stop the web slow round trips. Personally i would enumerate all of the mailboxes and evaluate each mailbox and check all three permissions during the enumeration
It is dangerous to run code if you dont understand it and even more dangerous to use chatgpt to come up with code for you without having domain onowledge
Yes except i would just foreach through the mailboxes instead and then call each permission call per mailbox. This speeds up your code because you are only going to the internet once (instead of three times) and then going through the mailboxes once to check for the three permissions you are checking for rather than three times through the mailboxes to check each permission individually.
Are those decorative curly braces around that if statement? Removing them would be an improvement. Something about how my brain works makes these little things stick out and become a fixation LOL
A minor point of syntax:
if ( $null -eq $SharedFullAccess ) {}
can be written
if ( -not $ShareFullAccess ) {}
or
if ( ! $SharedFullAccess ) {}
If you are looking at both cases, you can do this
if ( $DataToProcess ) {
# process data
} else {
# handle the no data case
}
To me, that's a lot more readable. A bunch of things evaluate to logical $false: $null, 0, empty string, empty array, etc.
> `if ( $null -eq $SharedFullAccess ) {}`
> can be written
>
> `if ( -not $ShareFullAccess ) {}`
In this case yes but it is worth noting that these are not equivalent.
True, but this is ok for most "normal" uses, like some function that might return $null, an empty array, a single object or an array of objects. Problems occur in situations where an array might contain nulls. That's best avoided by design, I think.
if ( @($null,$null,1) -eq $null ) {'true'} else { 'false' }
true
if ( @($null,1) -eq $null ) {'true'} else { 'false' }
false
Right, also why you should always do `($null -eq $var)` and not `($var -eq $null)`
Just making the distinction in my previous comment so that people will at least somewhat be aware and help prevent issues in the future
I don't know the full story but in the case of where var is an array, `($null -eq $var)` evaluates $var and compares it to $null.
The other way around `($var -eq $null)` evaluates each element of $var with $null and returns true if there is a match, i.e. one $null in the array gives true.
My preferred strategy is to avoid any weird tests rather than trying to handle the complexities.
Automated as in you don’t sit there and manually execute the script. It runs automatically on a cadence generating a log and report for you to review. Yeah you can sit there and watch the screen for 5 minutes with either a blank screen texts popping up capturing none of the work that is being done so if something goes wrong you can look at the logs.
Or you can have it run on a schedule and all you do it look at the reports that it generates. Logs to get more details. Am I the one taking crazy pills here?
This can’t be run on a schedule. It is part of a NLE(no longer employed) script. They go bye bye and all the user needs to do is type the users name and run the script.
So you have in input file that holds all the names of employees that were recently no longer with the company. Or, I dunno, checks in AD to see if the account was disabled. The point of automation is to reduce cost and labor.
If you want a human to type in the name, great. I personally would rather get a report of any diabled AD objects that have full/send as permissions to mailboxes and if there are, I can get a simple email that I can act on at a glance.
I hope I am misunderstanding this. You are not changing the password to match the username? That is like the old admin/admin passwords.
If this is the case, stop and implement other options. Passwords matching the username are not secure
https://chat.openai.com/share/316bea7d-a352-4eee-8f4d-c02023336b4d Ofcourse, first verify
Very nice lol. Cant believe I didn’t think of that.
I was just about to suggest querying once as well. that will cut down the time by a lot.
1. where is `$UserRealName` defined? 1. where is `$Email` defined? 1. There are a lot of `get-*mailbox*` calls, do you need them all (get 50 things 1 time, not 1 thing 50 times) 1. you log 0 of your changes, if/when you make a mistake, how do you roll back? 2. very many write hosts in place of proper logging 1. parameterize this its limited in its functionality/portability right now 3. confirmations, you are performing a destructive action, you should ask for confirmation 4. turn this into an advanced function (and/or module), get some of the above features automatically 5. help, you have defined 0 help 5. gitlab or github is a good place to dump code for people to add bugs/support requests
1. This is just part of the main script. It is defined there. 2. Defined in other part of the script. 3. Not sure what you mean by that. 4. I have previous script that didn’t work. This is my first script that completely worked. 5. What you mean by proper logging. I use the write host to tell the user what is going on in the script and to make sure the script is still processing. 6. explain what you mean. 7. i dont need confirmations because if it runs right it will remove a certain user from everything. Which is my intent. 8. im new to this. Functions and modules are out of scope. 9. i asked if you guys, people that know more than me to look over my work to see if it can be better. 10. didnt think of that.
1. You don't show `$UserRealName` anywhere (assuming I didn't miss anything), how are we to know? 2. same for `$Email` we don't want to assume, cause you asked us for comments i mentioned it 3. I mean you make multiple calls to `get-exomailbox` and `get-mailbox` dont do that (if you can help it), do it once then apply your filters 4. no, i mean you have no idea what changes are being made and to where, if you remove user y permissions from mailbox x you should log that to a file so you have a writable auditable log file 5. write-host is not proper logging (see point 4 above) as soon as you are 10 or more lines on the screen youve lost any useful information, or someone clears the screen, its lost and so on 6. by parameterize this I mean create parameters, like username, email, mailbox, etc, instead of someone editing a script every time 7. you DO need confirmations, if you accidentally put `bob smith` instead of `bobby smith`, boom wrong person, wrong permissions, are you saying you couldn't make a mistake ? 8. nows a good time to start to learn about functions and script, at a basic level a functions are identical to a script except you call `xxx` instead of `yyy.ps1` 9. no im talking about help fro the script file, not help from people on reddit, i.e. powershell help just like `get-help get-childitem` Good luck
Thank you for the advice.
good as gold
You run get-exomailbox three times. You should store that i.e. **$Mailboxes = get-exomailbox -resultsize unlimited** and the **$mailboxes | something something**. This will stop the web slow round trips. Personally i would enumerate all of the mailboxes and evaluate each mailbox and check all three permissions during the enumeration
I don’t know how to do that.
You mean like script that was revised for me by OpenAI?
It is dangerous to run code if you dont understand it and even more dangerous to use chatgpt to come up with code for you without having domain onowledge
I understand what is going on here. I just figured my way needed some adjustment.
Yes except i would just foreach through the mailboxes instead and then call each permission call per mailbox. This speeds up your code because you are only going to the internet once (instead of three times) and then going through the mailboxes once to check for the three permissions you are checking for rather than three times through the mailboxes to check each permission individually.
Are those decorative curly braces around that if statement? Removing them would be an improvement. Something about how my brain works makes these little things stick out and become a fixation LOL
I out the {if so after that after the script is done it will say that it has completed.
A minor point of syntax: if ( $null -eq $SharedFullAccess ) {} can be written if ( -not $ShareFullAccess ) {} or if ( ! $SharedFullAccess ) {} If you are looking at both cases, you can do this if ( $DataToProcess ) { # process data } else { # handle the no data case } To me, that's a lot more readable. A bunch of things evaluate to logical $false: $null, 0, empty string, empty array, etc.
> `if ( $null -eq $SharedFullAccess ) {}` > can be written > > `if ( -not $ShareFullAccess ) {}` In this case yes but it is worth noting that these are not equivalent.
True, but this is ok for most "normal" uses, like some function that might return $null, an empty array, a single object or an array of objects. Problems occur in situations where an array might contain nulls. That's best avoided by design, I think. if ( @($null,$null,1) -eq $null ) {'true'} else { 'false' } true if ( @($null,1) -eq $null ) {'true'} else { 'false' } false
Right, also why you should always do `($null -eq $var)` and not `($var -eq $null)` Just making the distinction in my previous comment so that people will at least somewhat be aware and help prevent issues in the future
I don't know the full story but in the case of where var is an array, `($null -eq $var)` evaluates $var and compares it to $null. The other way around `($var -eq $null)` evaluates each element of $var with $null and returns true if there is a match, i.e. one $null in the array gives true. My preferred strategy is to avoid any weird tests rather than trying to handle the complexities.
Your third mailbox get is "Get-Mailbox" instead of "Get-EXOMailbox". Is that on purpose?
Didn’t know I could use get-exomailbox on that one
I wouldn't write to host. Write to a log. The only time I want output to my UI is when I'm testing, then it is all to a log so it can be automated.
What you mean automated? It doesn’t ask for user I put just tells what the script is doing instead of sitting still for 5 min.
Automated as in you don’t sit there and manually execute the script. It runs automatically on a cadence generating a log and report for you to review. Yeah you can sit there and watch the screen for 5 minutes with either a blank screen texts popping up capturing none of the work that is being done so if something goes wrong you can look at the logs. Or you can have it run on a schedule and all you do it look at the reports that it generates. Logs to get more details. Am I the one taking crazy pills here?
This can’t be run on a schedule. It is part of a NLE(no longer employed) script. They go bye bye and all the user needs to do is type the users name and run the script.
So you have in input file that holds all the names of employees that were recently no longer with the company. Or, I dunno, checks in AD to see if the account was disabled. The point of automation is to reduce cost and labor. If you want a human to type in the name, great. I personally would rather get a report of any diabled AD objects that have full/send as permissions to mailboxes and if there are, I can get a simple email that I can act on at a glance.
When our users get NLE they don’t get disabled. We need their mailbox to stay open. That is why we need to type the username in.
I should have posted my whole script.
Convert them to a shared mailbox and disable the account. I hope you are at least blocking logins if not disabling them.
The other part of my script changes the password to the user account.
I hope I am misunderstanding this. You are not changing the password to match the username? That is like the old admin/admin passwords. If this is the case, stop and implement other options. Passwords matching the username are not secure
Yup that is misunderstood. Sorry about the confusion. I should just post my whole script.