microsoft / FastTrack

GitHub home for Microsoft FastTrack
MIT License
159 stars 100 forks source link

Issue with parameter ExcludeServiceAcctsCSV in Find-MailboxDelegates #163

Closed bezibaerchen closed 1 year ago

bezibaerchen commented 1 year ago

Dear FastTrack team,

we are currently planning our migration and have a longer list of accounts that should not be analyzed or included into batches.

For that purpose we created a file named "exclude.csv" containing no headers and one primary SMTP per line.

However, after the script finishes running all of those accounts are still included in batches and permission analysis.

Anything we potentially missed?

bezibaerchen commented 1 year ago

Addition:

Script is called like this:

\Find-MailboxDelegates.ps1 -SendAs -EnumerateGroups -SendOnBehalfTo -Calendar -ExcludeServiceAcctsCSV "C:\scripts\ps\_ExchangeOnline\exclude.csv"

soyalejolopez commented 1 year ago

Hi @bezibaerchen , can you check the log file for any issues when excluding the service accounts? The log file should be located here:

$LogFile = "$scriptPath\Find-MailboxDelegates-$yyyyMMdd.log"

Glad to take a look as well if you're okay with sharing.

bezibaerchen commented 1 year ago

Hi @bezibaerchen , can you check the log file for any issues when excluding the service accounts? The log file should be located here:

$LogFile = "$scriptPath\Find-MailboxDelegates-$yyyyMMdd.log"

Glad to take a look as well if you're okay with sharing.

Unfortunately nothing obvious. Only occurence is with "passed script parameters".

In which of the phases should that be referenced?

Unfortunately can't share the file as it contains more or less all of our mail addresses :-)

soyalejolopez commented 1 year ago

Completely understand :) It's hard to say what might be going wrong with the current info. One suggestion here, would be to temporarily write out the excluded service accounts:

$ExcludedServiceAccts

within the Get-Permissions() function in Line 251.

If that does dump the list of service accounts, then the issue is likely in the comparison logic (if/else). Otherwise, there might be an issue with the way we're collecting the list of service accounts.

bezibaerchen commented 1 year ago

Maybe I am also misinterpreting the way that this switch works. How would the script behave if a mail address is found in that CSV? Shall it not be added to any batch or will it end up in no-dependencies? I would have expected that it doesn't even analyze and not add them to any batch but I might be wrong.

bezibaerchen commented 1 year ago

Addition: list of accounts is dumped correctly.

soyalejolopez commented 1 year ago

It'll skip the mail address when it's collecting permissions. The batching uses the permissions found, so that service account would not be a part of the analysis.

Assuming the list of accounts is getting dumped correctly within the function, then something might be off with the comparison logic. Line 371 for example:

if(-not ($ExcludedServiceAccts -contains $usrTmpEmail -or $ExcludedServiceAccts -contains $mailbox.primarySMTPAddress.ToString()))

You could add $usrTmpEmailand $mailbox.primarySMTPAddress.ToString() within that "if" statement to see if the condition evaluated to "true" which it shouldn't have if it was in the excluded service accounts list.

bezibaerchen commented 1 year ago

Could you precise what to add where and what to look for?

bezibaerchen commented 1 year ago

We added those 2 variables to the write-log entry. Doesn't seem to match so line 371 obviously never turns true.

For whatever reason script always seems to jump into the else in line 377

soyalejolopez commented 1 year ago

Could you precise what to add where and what to look for?

Within the if statement that's on Line 371, add both $usrTmpEmail and $mailbox.primarySMTPAddress.ToString() in there to see if it dumps out your service accounts. If it does it means that the condition on Line 371 resulted in "true" even when there's a service account, which is unexpected.

We added those 2 variables to the write-log entry. Doesn't seem to match so line 371 obviously never turns true. For whatever reason script always seems to jump into the else in line 377

Line 371 is a nested if condition which doesn't have a matching else statement. Whenever there's a service account found, it should skip adding the permission, ie the if condition should return false and not collect permission.

image

bezibaerchen commented 1 year ago

Still kinda lost :-) Would we expect all accounts from service account csv to show up in no dependencies batch or nowhere?

soyalejolopez commented 1 year ago

No worries, let me know where I can guide more and happy to expand.

To answer your question, service accounts should not show up in any of the outputs, including dependencies/batching, permissions.

bezibaerchen commented 1 year ago

Hi soya, we've been playing around in the code but can't come to a conclusion. We still see users in excluded service accounts file being processed and added to batches, namely in nodependencies one. Can you maybe try to reproduce on your end? Or maybe we can somehow have joint session.

soyalejolopez commented 1 year ago

Hi @bezibaerchen , let me see if I can get a repro internally. What exchange on prem version are you using?

soyalejolopez commented 1 year ago

@bezibaerchen - I was able to spin up an Exchange 2016 environment. See below screenshot of what you should expect to see.

image

bezibaerchen commented 1 year ago

Okay, then this is clarified that excluded mailboxes are not being analyzed and put into nodependencies. Thanks for your efforts, highly appreciated!