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

Update Get-AzureADLogs.ps1 #69

Closed angry-bender closed 6 months ago

angry-bender commented 6 months ago

Fixed Interval field in AzureAdSignInLogs Acquisition

Added the split by time feature to Get-AzureADAuditLogs, interval 12 hours (Larger dataset than SignInLogs).

invictus-korstiaan commented 6 months ago

Thanks for the PR, will test it soon and see if we can merge it.

JoeyInvictus commented 6 months ago

Hi, thanks again for the pull request, it looks good and hopefully fixes some of the issues.

I encountered some small errors while running the code. Maybe you already fixed them in your fork, but if not, let me know and I will try to fix them before accepting the pull request.

For the Get-ADSignInLogs functionality it seems to fail due to a syntax error in the filter line.

image

I had to change the following two variables troughout the script to get it working:

$currentStart.ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ssZ")
$currentEnd.ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ssZ")

Instead of yyyy-MM-dd HH:mm:ss, I had to use yyyy-MM-ddTHH:mm:ssZ.

Is this also the case for you? Or does it work for you without the "T" and "Z" in the date?

For the Get-ADAuditLogs, there seems to be an error in the date lines with the duplicate 'yyyy-MM-dd HH:mm:ss' as shown in the screenshot below.

image

For the Get-AdAuditLogs I also had to change the date format by adding a T and Z.

In addition, the line below seems to be out of place, causing an error due to an unexpected token '}' in the expression or statement.

Write-LogFile -Message "[INFO] Acquisition complete, check the $($OutputDir) directory for your files.." -Color "Green"
}

image

Another small one, need to change 1440 to 720 in the write-logfile.

if ($Interval -eq "") {
        $Interval = 720
        Write-LogFile -Message "[INFO] Setting the Interval to the default value of 1440 (Larger values may result in out of memory errors)"
    }
angry-bender commented 6 months ago

Hi, thanks again for the pull request, it looks good and hopefully fixes some of the issues.

I encountered some small errors while running the code. Maybe you already fixed them in your fork, but if not, let me know and I will try to fix them before accepting the pull request.

For the Get-ADSignInLogs functionality it seems to fail due to a syntax error in the filter line.

image

I had to change the following two variables troughout the script to get it working:

$currentStart.ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ssZ")
$currentEnd.ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ssZ")

Instead of yyyy-MM-dd HH:mm:ss, I had to use yyyy-MM-ddTHH:mm:ssZ.

Is this also the case for you? Or does it work for you without the "T" and "Z" in the date?

For the Get-ADAuditLogs, there seems to be an error in the date lines with the duplicate 'yyyy-MM-dd HH:mm:ss' as shown in the screenshot below.

image

For the Get-AdAuditLogs I also had to change the date format by adding a T and Z.

In addition, the line below seems to be out of place, causing an error due to an unexpected token '}' in the expression or statement.

Write-LogFile -Message "[INFO] Acquisition complete, check the $($OutputDir) directory for your files.." -Color "Green"
}

image

Another small one, need to change 1440 to 720 in the write-logfile.

if ($Interval -eq "") {
      $Interval = 720
      Write-LogFile -Message "[INFO] Setting the Interval to the default value of 1440 (Larger values may result in out of memory errors)"
  }

I'll double check and compare to my local copy shortly

angry-bender commented 6 months ago

That should fix the issues listed above, apologies on that one

JoeyInvictus commented 6 months ago

Hi, thanks! It looks good :). I had to fix a small error with the date format for the Audit functionality.

$($currentStart.ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ssZTHH:mm:ssZ")) The time was included twice, which resulted in an error.

angry-bender commented 6 months ago

Hi, thanks! It looks good :). I had to fix a small error with the date format for the Audit functionality.

$($currentStart.ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ssZTHH:mm:ssZ")) The time was included twice, which resulted in an error.

Thank you 😊