Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Get-AzureADLogs.ps1 #69

Merged
merged 7 commits into from
May 22, 2024
Merged

Conversation

angry-bender
Copy link
Contributor

Fixed Interval field in AzureAdSignInLogs Acquisition

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

Fixed Interval field in AzureAdSignInLogs Acquisition

Added the split by time feature to Get-AzureADAuditLogs, interval 12 hours (Larger dataset than SignInLogs).
@angry-bender angry-bender marked this pull request as ready for review May 10, 2024 03:32
Added more verbose logging
@invictus-korstiaan
Copy link
Contributor

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

@JoeyInvictus
Copy link
Collaborator

JoeyInvictus commented May 21, 2024

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@JoeyInvictus
Copy link
Collaborator

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.

@JoeyInvictus JoeyInvictus merged commit 29aa9fa into invictus-ir:main May 22, 2024
@angry-bender
Copy link
Contributor Author

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 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants