Support export logging #386

superlinkx · 2024-02-02T21:46:23Z

Description

Add support for conditional auditing
Add support for including RequestURL in audit logs
Add better support for pprof
Fix potential for deadlocking in analysis

Motivation and Context

Required for some BHE related audit logging. Will likely come in handy as we expand audit logging throughout BHCE as well

How Has This Been Tested?

Local testing shows these changes are working as expected
All automated tests run successfully

Types of changes

Chore (a change that does not modify the application functionality)
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

Documentation updates are needed, and have been made accordingly.
I have added and/or updated tests to cover my changes.
All new and existing tests passed.
My changes include a database migration.

…of a generic response

… on the presence of specific context This commit mostly contains the logic to make this method for audit logging work. The only audit log event that works in this commit is CreateUser

…ure cases.

* updated IP address parsing * added tests for IP parsing, changed IP delimiter to comma

…elds

* chore: clean up old approach to reduce noise * chore: cleanup audit logging interface for new usages * chore: fix existing tests (audit integration test needs new target) * chore: improve integration config defaults * feat: add audit logging to UpdateUser This adds a new `AuditableContext` method to the Database, which wraps the Gorm Transaction method with some auditing context and ensures the two phase commit of the audit log is run * feat: add audit logging to DeleteSAMLProvider fix: use actual AuditData instead of the direct model * feat: CreateAssetGroup and DeleteAssetGroup are now back to auditable fix: broken tests * feat: plumb commitID for real this time fix: incorrect formatting of audit log action names fix: additional test fixes * fix: additional test fixes * review comments --------- Co-authored-by: Irshad Ahmed <[email protected]>

* BED-3858 - Fix not ingesting tenant count during azure analysis (#328) Co-authored-by: Irshad Ahmed <[email protected]> * ESC9a Edge Composition (#354) * feat: esc9a post * test: add esc9 test * chore: add harness files * fix: regen schema after merge * chore: fix small nits * chore: cleanup cert template new function * chore: add missing props * wip: 9a composition * fix: treat failure to grab properties as true * wip: esc9a composition * wip: esc9a composition * feat+chore: add depth controls to dawgs patterns * wip: esc9a composition * fix: do not drop the current segment if the next pattern is optional * wip: esc9a composition * fix: update other continuations to respect depth correctly * wip: edge comp * fix: swap * chore: remove unnecessary logs * feat: esc9a post * test: add esc9 test * chore: fix small nits * wip: 9a composition * wip: esc9a composition * wip: esc9a composition * feat+chore: add depth controls to dawgs patterns * wip: esc9a composition * fix: do not drop the current segment if the next pattern is optional * wip: esc9a composition * fix: update other continuations to respect depth correctly * wip: edge comp * fix: swap * chore: remove unnecessary logs * test: add test covering esc9a edge comp * chore: revert random re-ordering * chore: handle negative min/max depth on continuations --------- Co-authored-by: John Hopper <[email protected]> * docs: Add to ESC3 abuse info (#350) * docs: add note in ESC6 abuse info (#356) * feat: Add ADCS pre-built queries (#342) Co-authored-by: Rohan Vazarkar <[email protected]> * feat: esc6a edge composition (#359) * feat: esc6a edge composition * chore: allow composition accordion to show for 6a * fix: add trustedby rel to path4 pattern, use outboundwithdepth for optional memberof traversal * chore: update dcfor pattern to use outboundwithdepth for optional group membership * ESC10a Post Processing (#360) * wip: initial ESC10a post * test: all the tests for esc10a * chore: add edges to post processed * chore: add harnessgen script * test: remove edges from harness * chore: don't exit loop if we hit an error, continue instead * chore: log and continue * feat: filter out ESC3 false positives (#351) * feat: filter out ESC3 false positives * fix: handle esc3 filtering without retraversal * fix: handle esc3 filtering without retraversal * fix: handle esc3 filtering without retraversal * chore: rename function for re-use * chore: log and continue --------- Co-authored-by: rvazarkar <[email protected]> Co-authored-by: Rohan Vazarkar <[email protected]> * chore: patch EULAAcceptance bypass to only run if the current user is set to false * fix: incorrect usage of RemoteAddr fix: unnecessary AuditData() calls fix: use pointers for AuditEntry.Model assignments so successful actions can record updated fields like ID --------- Co-authored-by: mistahj67 <[email protected]> Co-authored-by: Irshad Ahmed <[email protected]> Co-authored-by: Rohan Vazarkar <[email protected]> Co-authored-by: John Hopper <[email protected]> Co-authored-by: Jonas Bülow Knudsen <[email protected]> Co-authored-by: Ulises Rangel <[email protected]> Co-authored-by: rvazarkar <[email protected]>

* audit logs for auth tokens and secrets * fix bad copy

…elds

…ector, CreateSAMLIdentityProvider, and UpdateSAMLIdentityProvider have audit log support (#374) chore: remove unused RemoveAssetGroupSelector method chore: update tests and mocks to account for interface changes

* more auth handlers for audit log * minor optimization

…status

* feat: create an audit log record when unauthorized access is attempted * chore: Only log unauthorized write access * chore: add audit logging to remaining auth middleware * test: Fix middleware/auth_test to support new audit log changes

It was previously sized to only account for max 1 IPv6 address, but our design changes mean it could now be any length

This reverts commit 2a2c839.

…ostgres

* Refactored unauthorized access audit logging: - moved audit logging responsibility to Authorizer interface - cleaned up injections of *db through several layers of code (no longer necessary) - minor cleanup and support work

… in deep audit logging

…etimes auditable

…t of endpoints

…ding a potential channel race

superlinkx · 2024-02-05T20:02:28Z

cmd/api/src/api/middleware/middleware.go

@@ -139,7 +140,8 @@ func ContextMiddleware(next http.Handler) http.Handler {
 					Scheme: getScheme(request),
 					Host:   request.Host,
 				},
-				RequestIP: parseUserIP(request),
+				RequestedURL: model.AuditableURL(request.URL.String()),


Pull the request URL for auditing purposes

superlinkx · 2024-02-05T20:04:30Z

cmd/api/src/daemons/api/toolapi/api.go

Replace our bespoke trace endpoint in the toolapi with the proper pprof compatible tools for easier use.

EDIT: added the old trace endpoint back due to how Acumen interacts with it. Will update acumen later and then properly remove this endpoint

superlinkx · 2024-02-05T20:05:01Z

cmd/api/src/database/audit.go

Kind of a dumb name, but allows us to add conditional audit logging a bit easier

superlinkx · 2024-02-05T20:05:33Z

cmd/api/src/model/audit.go

AuditableURL was needed to support passing the request url to the audit log logic that requires an Auditable interface

superlinkx · 2024-02-05T20:06:51Z

packages/go/analysis/ad/membership.go

Work by @zinic to plug a deadlock condition I hit locally. This cleans up the logic of this function in general, but should be reviewed carefully to ensure nothing was missed. Tests fine locally now

superlinkx · 2024-02-05T20:07:02Z

packages/go/slicesext/slicesext_test.go

Remove dead code

cmd/api/src/database/audit.go

…tes to acumen and we can't wait on those

juggernot325

LGTM

juggernot325 and others added 30 commits January 25, 2024 16:40

feat: Initial migration and model updates for AuditLog improvements

0e1d356

feat: populate actor_email for existing audit_logs

d0c6173

chore: Fix audit log docs to show the actual response object instead …

1d08960

…of a generic response

feat: Log Requester IP for source and actor email with audit logs

6019c99

wip: createUser audit logging via middleware

0f0d55a

chore: refactor audit logging to trigger from LoggingMiddleware based…

3d65a85

… on the presence of specific context This commit mostly contains the logic to make this method for audit logging work. The only audit log event that works in this commit is CreateUser

wip: Successfully creating audit logs from middleware. Capturing fail…

86b77a3

…ure cases.

feat: Capture DB errors in audit logs

22c3e8c

chore: remove debug logging

cf4b854

chore: Refactor setting of status to receiver function

985d580

wip: All "simple" audit context endpoints updated for middeware

29673a9

fix: Fix a few bugs found during testing

a05ba51

fix: Remove doc for non-existent POST /api/v2/saml endpoint

cd78c04

wip: Refactor to simplify setting of AuditCtx in endpoints

9cb8a3d

fix: registration issue after rebase

e1d81bd

tasks 6 and 7 for #345 (#357)

3ac13c1

* updated IP address parsing * added tests for IP parsing, changed IP delimiter to comma

Merge remote-tracking branch 'origin/main' into populate-audit-log-fi…

fd32507

…elds

redo IP parsing for changed requirements (#362)

2a14ff5

add eula_accepted to user audit data (#371)

6810bd3

audit logs for auth tokens and secrets (#372)

0482b61

* audit logs for auth tokens and secrets * fix bad copy

Merge branch 'main' into populate-audit-log-fields

af01314

Merge remote-tracking branch 'origin/main' into populate-audit-log-fi…

9ff9831

…elds

more auth handlers for audit log (#373)

d744c31

* more auth handlers for audit log * minor optimization

chore: update documentation for our new commit_id field and intent …

061dc91

…status

chore: change source column to TEXT

13f4160

It was previously sized to only account for max 1 IPv6 address, but our design changes mean it could now be any length

rename audit log source column, add indices

2a2c839

juggernot325 and others added 11 commits February 1, 2024 12:44

Revert "rename audit log source column, add indices"

cc16a11

This reverts commit 2a2c839.

chore: rename audit_log.source as 'source' is a reserved keyword in p…

2624969

…ostgres

chore: add additional fields to user.AuditData

c36c609

Refactored unauthorized access audit logging: (#381)

eb5e741

* Refactored unauthorized access audit logging: - moved audit logging responsibility to Authorizer interface - cleaned up injections of *db through several layers of code (no longer necessary) - minor cleanup and support work

chore: add RequestedPath to BHCtx to ensure we can reflect on the URL…

9c3b76b

… in deep audit logging

Merge remote-tracking branch 'origin/main' into support-export-logging

0330679

feat: MaybeAuditableTransaction added to support methods that are som…

de6bc2e

…etimes auditable

chore: cleanup before review

0719913

Merge branch 'main' into populate-audit-log-fields

847c92c

chore: final redesign for RequestedURL to support Auditable interface

ed025f5

Merge branch 'populate-audit-log-fields' into support-export-logging

8ec67ab

Base automatically changed from populate-audit-log-fields to main February 5, 2024 18:31

superlinkx added 5 commits February 5, 2024 12:26

chore: move our toolapi trace handler to a proper pprof compatible se…

5ff9e33

…t of endpoints

fix: changes from @zinic to clean up ResolveAllGroupMemberships, avoi…

b39f476

…ding a potential channel race

Merge remote-tracking branch 'origin/main' into support-export-logging

0185b1e

chore: remove commented out code again

7e15c49

chore: remove unused code

c5b2fd3

superlinkx commented Feb 5, 2024

View reviewed changes

superlinkx marked this pull request as ready for review February 5, 2024 20:11

Merge remote-tracking branch 'origin/main' into support-export-logging

9874e26

irshadaj reviewed Feb 5, 2024

View reviewed changes

cmd/api/src/database/audit.go Show resolved Hide resolved

superlinkx added 2 commits February 5, 2024 15:14

revert: bring back the old tracing endpoint, since it'll require upda…

deafa44

…tes to acumen and we can't wait on those

Merge remote-tracking branch 'origin/main' into support-export-logging

6666105

juggernot325 approved these changes Feb 6, 2024

View reviewed changes

superlinkx merged commit 9339c3d into main Feb 6, 2024
3 checks passed

superlinkx deleted the support-export-logging branch February 6, 2024 18:55

github-actions bot locked and limited conversation to collaborators Feb 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support export logging #386

Support export logging #386

superlinkx commented Feb 2, 2024 •

edited

Loading

superlinkx Feb 5, 2024

superlinkx Feb 5, 2024 •

edited

Loading

superlinkx Feb 5, 2024

superlinkx Feb 5, 2024

superlinkx Feb 5, 2024

superlinkx Feb 5, 2024

juggernot325 left a comment

Support export logging #386

Support export logging #386

Conversation

superlinkx commented Feb 2, 2024 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

superlinkx Feb 5, 2024

Choose a reason for hiding this comment

superlinkx Feb 5, 2024 • edited Loading

Choose a reason for hiding this comment

superlinkx Feb 5, 2024

Choose a reason for hiding this comment

superlinkx Feb 5, 2024

Choose a reason for hiding this comment

superlinkx Feb 5, 2024

Choose a reason for hiding this comment

superlinkx Feb 5, 2024

Choose a reason for hiding this comment

juggernot325 left a comment

Choose a reason for hiding this comment

superlinkx commented Feb 2, 2024 •

edited

Loading

superlinkx Feb 5, 2024 •

edited

Loading