Refactor desktop audit event emission

[zmb3]

Our methods for emitting audit events take 10 arguments already, and we need to add more as part of the work in #30417. To make this more manageable, create an auditor struct that will hold on to state that is shared for all audit events in a session (ID, user identity, the desktop we're connecting to, etc.)

As a result, the "audit cache" for directory sharing events is also simplified - we now create one of these per-session rather than maintaining one large cache for all sessions.

[zmb3]

Convention: methods starting with make are just constructors - they always return an event.

Methods starting with on may or may not return an event (some of these just update the internal state for future events).

[zmb3]

Note to self to inline this.

[zmb3]

This is the new type which holds on to a lot of common state (so that we don't have to pass these values around every time we want to emit an event).

The other change worth noting is this thing only build events - it's up to the caller to decide whether the event gets emitted to the audit log, recorded to the session recording, or both.

[zmb3]

We've removed one layer of indirection here. An audit cache is associated to a single session, so the top level map of session ID to entry no longer needs to exist.

(And all the method signatures have been updated to remove the session ID argument)

[zmb3]

This callback is now purely authorization and no longer grabs the windowsUser variable - we get that from the client instead.

(There was actually a subtle bug here in that sometimes we would try to emit audit events before we had a Windows username.)

[zmb3]

Closing the connection on error was moved from audit.go (which is just about building audit events) to windows_server.go (where the rest of the connection management logic is.

[zmb3]

This test was moved to audit_test.go.

[zmb3]

Most of these tests got a bit simpler, as we no longer need to emit the event and then ask a mock emitter what was emitted, we just build the event and it's already the type we expect.

[zmb3]

The changes in this file look scary, but it's very mechanical.

• moved a bunch of constants to the top rather than repeat them dozens of times
• no longer need to build send/receive handlers and encode TDP messages - instead we operate on the new audit event builder in a strongly-typed manner

[ibeckermayer]

Logs are working for me

[rosstimothy]

I'm not super familiar with this code but isn't there potential for the username to empty if this is called before the username has been read and assigned in readClientUsername below? What are the implications if that happens?

[zmb3]

Yep, there is the potential. The implications are that the username would be missing from the audit event. This would only be the case if we failed to initialize a client in this branch:

https://github.com/gravitational/teleport/pull/33189/files#diff-d0d78875d7396d7dad9cc4ab10f6da26554c8e8ff30f2b661c1377d9b959bd85R864-R867

[public-teleport-github-review-bot]

@zmb3 See the table below for backport results.

Branch	Result
branch/v12	Failed
branch/v13	Failed
branch/v14	Create PR

[zmb3]

Going to abandon the v13/v12 backports. I was only backporting this to those versions to try to avoid future conflicts, but there's conflicts here already.

Edit: I want to see Telemetry for v13 agents, which will still be around for a bit, so I opened a manual v13 backport.