invictus-ir / Microsoft-Extractor-Suite

A PowerShell module for acquisition of data from Microsoft 365 and Azure for Incident Response and Cyber Security purposes.
https://microsoft-365-extractor-suite.readthedocs.io/en/latest/
GNU General Public License v2.0
481 stars 68 forks source link

Make AzureADGraphLogs log a bit more verbose #94

Closed Matthijsy closed 1 month ago

Matthijsy commented 1 month ago

Currently both commandlets within the AzureADGraphLogs (Sign-in and Audit) keep logging that they created a file. However, this doesn't give you any information about how long you can expect it to run. This PR adds a part to the log which indicates the amount of records, and the timeframe of records that have just been written to disk. This allows you to better see progress, and have an idea how much still needs to be collected.

Furthermore, since we now know which timeframe we have collected, and which still need to be processed we can calculate progress. So I've also added a little progress bar which shows the percentage

JoeyInvictus commented 1 month ago

Hi, thanks for the pull request, we really appreciate the contribution! I like the idea of adding more progress information, and I’ll explore incorporating something similar into other scripts like Get-UALGraph for the next update.

I tried applying the pull request, but I'm encountering issues with the .ToString('yyyy-MM-ddTHH:mmZ') part. It throws the following error:

image

When I remove the .ToString part, everything seems to work fine.

$from =  ($dates | Select-Object -First 1).ToString('yyyy-MM-ddTHH:mmZ')
$to = ($dates | Select-Object -Last 1).ToString('yyyy-MM-ddTHH:mmZ')

image

Does it work with the .ToString for you?

Matthijsy commented 1 month ago

Thank you for the quick check! It does work with the ToString for me, if I remove that it starts to display it in the US format:

[INFO] Sign-in logs written to Output/AzureAD/20240926-SignInLogs/20240926163314-SignInLogsGraph.json (1000 records between 09/26/2024 13:31:56 and 09/26/2024 14:28:58)

I do use PowerShell on mac, so that might be the difference. However, the ToString part is used in more parts of the code, so quite weird that it does not work for this part.

JoeyInvictus commented 1 month ago

Mhh interesting. I’ll take a look at it tomorrow, dates in PowerShell never seem to make things easy.

JoeyInvictus commented 1 month ago

I got it working by converting the $_.CreatedDateTime property into a DateTime object. The updated line looks like this:

$dates = $responseJson.value | ForEach-Object { [DateTime]::Parse($_.CreatedDateTime) } | Sort-Object

One minor thing with the Audit Log, there's an unnecessary closing parenthesis at the end of this line: Write-LogFile -Message "[INFO] Audit logs written to $filePath ($count records between $fromstr and $to))"

I have a question regarding the suggested change below. I was wondering why you want to switch it to Write-Host. Are you encountering issues on a MacBook? We intentionally changed it to [Console]::WriteLine($message) because it's slightly faster and is used a lot across different functionalities.

This is especially helpful for the Get-UALAll part, as it prints a large number of lines to the console. For instance, if you're retrieving 1 million logs with a maximum of 5,000 per interval, you would need to loop 200 times, printing at least two lines to the terminal for each loop. By saving a bit of time on each line printed and doing this 400 times, you can shave off some extra time during the acquisition. Still the acquistion takes way to long tbh.

image

Matthijsy commented 1 month ago

Thank you for testing it! I have changed the For-Each to include the datetime parsing.

The reason for removing the [Console]::WriteLine($message) is that it doesn't work nice when you also use Write-Progress, the text of the console gets behind the text of the Write-Progress. I am not the best with powershell, so there might be another way around this to keep both the speed and the Write-Progress

JoeyInvictus commented 1 month ago

I'm also not very good with PowerShell. This project started small but kind of got out of hand, haha. I'm not sure what the best solution is without modifying the whole tool. It might be best to temporarily change:

Write-LogFile -Message "[INFO] Sign-in logs written to $filePath ($count records between $fromStr and $to)" -ForegroundColor Green

to:

Write-Host "[INFO] Sign-in logs written to $filePath ($count records between $fromStr and $to)" -ForegroundColor Green

This way, we can bypass the Console::WriteLine issues when used with Write-Progress, without removing it for the rest of the script. While thinking about a long term solution.

JoeyInvictus commented 1 month ago

Hi, thanks! I have accepted the pull request much appreciated! I'll think about a solution for the Write-Host vs [Console]::WriteLine($message) issue to prevent breaking Write-Progress in the long term.